Skip to content

CXH-1473: Add source_type fields to GroupTrait and AppTrait#848

Open
al-conductorone wants to merge 6 commits into
mainfrom
cxh-1473-add-source-type-fields-to-traits
Open

CXH-1473: Add source_type fields to GroupTrait and AppTrait#848
al-conductorone wants to merge 6 commits into
mainfrom
cxh-1473-add-source-type-fields-to-traits

Conversation

@al-conductorone
Copy link
Copy Markdown

Connectors can now report what kind of group or app each entry is (security group, distribution list, dynamic, etc.) so C1 stops dropping that classification when it pulls data from identity providers.

Add typed source_type metadata to two traits so IDP-supplied group/app
kind information can flow through the SDK instead of being dropped or
stuffed into the loose profile struct.

GroupTrait gets group_source_type (field 3) plus raw_group_source_type
(field 4); AppTrait gets app_source_type (field 6) and
raw_app_source_type (field 7). All four are additive optional string
fields with ignore_empty validation, so existing connectors keep working
unchanged.

For GroupTrait, expose a typed-string GroupSourceType vocabulary in
pkg/types/resource matching the RFC table (native, app_imported,
built_in, directory_synced, dynamic, distribution) plus the matching
WithGroupSourceType / WithRawGroupSourceType functional options. For
AppTrait, ship WithAppSourceType / WithRawAppSourceType as free-form
strings; a normalized app vocabulary is out of scope for this slice.

Tests lock the wire-format values of the group vocabulary and round-trip
both option pairs.
Replace the tautological TestGroupSourceTypeVocabulary (which asserted
GroupSourceType("native") == GroupSourceTypeNative) with
TestGroupSourceTypeWireFormat: marshal each constant through a real
proto Marshal/Unmarshal and assert the deserialized GroupTrait carries
the documented literal string. A constant rename or typo now trips the
test; same-value aliases still pass.

Add per-trait validation tests covering the four new fields against the
proto validate.rules (max_bytes 64 on normalized, 256 on raw,
ignore_empty=true). Empty strings pass; 65-byte normalized and 257-byte
raw values surface a GroupTraitValidationError / AppTraitValidationError
with the expected Field() string.

Add annotation round-trip tests for both traits: pack the trait into a
v2.Resource via annotations.New, read it back through GetGroupTrait /
GetAppTrait, and confirm the new source_type fields survive the
anypb.UnmarshalTo path.

On the proto/Go side, expand the AppTrait.app_source_type comment with
example values ("saml", "oidc", "scim") instead of the unanchored
"follow-up RFC" pointer, and document that WithAppSourceType is
deliberately free-form (unlike WithGroupSourceType) so the next reader
doesn't read the asymmetry as an oversight.
Drop the inline group source-type vocabulary list from the proto comment
(it duplicates the Go source of truth at pkg/types/resource and creates
a two-places-to-update sync risk). Replace with a single-line pointer to
GroupSourceType.

Drop the raw_group_source_type / raw_app_source_type proto comments
entirely. Both restated the field name; the typed/raw pairing is already
explained on GroupSourceType's doc comment in pkg/types/resource.

Trim the AppTrait.app_source_type comment to one line. Examples ("saml",
"oidc", "scim") were borderline normative without being constraints, so
they invited misuse.

Trim TestGroupSourceTypeWireFormat's header to a single line that says
what the test catches (constant rename or typo) rather than restating
what the body does.
@al-conductorone al-conductorone requested a review from a team May 21, 2026 17:39
@linear-code
Copy link
Copy Markdown

linear-code Bot commented May 21, 2026

CXH-1473

Comment thread pkg/types/resource/app_trait.go Outdated
@github-actions
Copy link
Copy Markdown
Contributor

ghost commented May 21, 2026

General PR Review: CXH-1473: Add source_type fields to GroupTrait and AppTrait

Blocking Issues: 0 | Suggestions: 0 | Threads Resolved: 0
Review mode: incremental since f08ceb9
View review run

Review Summary

The new commits add an AppSourceType type alias and update WithAppSourceType to accept AppSourceType instead of a plain string, addressing the previous review suggestion about consistency with GroupSourceType. A minor whitespace cleanup in version.go is also included. No new issues found.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

None.

Copy link
Copy Markdown
Contributor

@ghost ghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blocking issues found.

Copy link
Copy Markdown
Contributor

@ghost ghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blocking issues found.

Comment thread pkg/types/resource/app_trait.go
Introduce AppSourceType string alias so callers can no longer pass a
raw value to WithAppSourceType (or vice versa) without a compiler
error. No constants yet, no app source-type vocab is defined.

Also drop the stray blank lines in pkg/sdk/version.go.
Copy link
Copy Markdown
Contributor

@ghost ghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blocking issues found.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants