Skip to content

Migrate off deprecated trait profile/status attributes - #84

Merged
laurenleach merged 1 commit into
mainfrom
lauren/migrate-deprecated-trait-attrs
Aug 14, 2026
Merged

Migrate off deprecated trait profile/status attributes#84
laurenleach merged 1 commit into
mainfrom
lauren/migrate-deprecated-trait-attrs

Conversation

@laurenleach

Copy link
Copy Markdown
Contributor

baton-sdk v0.20.6 moved profile, status, and created_at off the trait
messages onto attributes on Resource, deprecating the trait-level options and
getters. staticcheck flags every remaining call with SA1019, so verify / lint
is red on main.

This migrates the connector to the resource-level API:

  • With{User,Group,Role,App}Profile -> WithResourceProfile
  • WithStatus / WithDetailedStatus -> WithResourceStatus
  • WithCreatedAt / WithSecretCreatedAt -> WithResourceCreatedAt
  • trait GetProfile() / GetStatus() reads -> the equivalent read on the resource

The option type changes from a *TraitOption to a ResourceOption, so the calls
move out of the trait slice and into the variadic tail of the New*Resource call.
The two status enums are numerically identical, so the values map 1:1. Non-deprecated
trait data (login, aliases, emails, secret type/expiry) is untouched.

No behavioural change intended: the deprecated options already populated the
resource-level fields. golangci-lint run ./... reports 0 issues after this
change, and the package tests pass.

baton-sdk v0.20.6 moved `profile`, `status`, and `created_at` off the trait
messages onto attributes on `Resource`, deprecating the trait-level options and
getters. staticcheck flags every remaining call with `SA1019`, so `verify / lint`
is red on `main`.

This migrates the connector to the resource-level API:

- `With{User,Group,Role,App}Profile` -> `WithResourceProfile`
- `WithStatus` / `WithDetailedStatus` -> `WithResourceStatus`
- `WithCreatedAt` / `WithSecretCreatedAt` -> `WithResourceCreatedAt`
- trait `GetProfile()` / `GetStatus()` reads -> the equivalent read on the resource

The option type changes from a `*TraitOption` to a `ResourceOption`, so the calls
move out of the trait slice and into the variadic tail of the `New*Resource` call.
The two status enums are numerically identical, so the values map 1:1. Non-deprecated
trait data (login, aliases, emails, secret type/expiry) is untouched.

No behavioural change intended: the deprecated options already populated the
resource-level fields. `golangci-lint run ./...` reports 0 issues after this
change, and the package tests pass.
Comment thread pkg/connector/helpers.go
rs.WithAccountType(v2.UserTrait_ACCOUNT_TYPE_HUMAN),
}, rs.WithAnnotation(annos))
},
rs.WithResourceCreatedAt(proto.GetCreatedTime().AsTime()), rs.WithAnnotation(annos))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Suggestion: When CreatedTime is unset, GetCreatedTime().AsTime() yields 1970-01-01, so the resource gets an explicit (and wrong) created_at rather than being left unset — HasCreatedAt() then reports true. This is pre-existing behavior carried over from WithCreatedAt, not a regression, but since the call is being rewritten it's a cheap place to guard: only append rs.WithResourceCreatedAt(...) when proto.GetCreatedTime() != nil. Same applies at line 59 for service accounts. (confidence: medium)

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Connector PR Review: Migrate off deprecated trait profile/status attributes

Blocking Issues: 0 | Suggestions: 1 | Threads Resolved: 0
Criteria: Criteria status: loaded .claude/skills/ci-review.md from trusted base f26c08c26ded.
Review mode: full
View review run

Review Summary

Scanned the full PR diff for security and correctness. The change is a single file (pkg/connector/helpers.go, 4 insertions / 4 deletions) moving rs.WithCreatedAt from the UserTraitOption slice to the resource-level rs.WithResourceCreatedAt variadic tail for both users and service accounts; I verified against the vendored SDK that this is behavior-preserving, since WithCreatedAt previously only reached Resource.created_at via the trait-mirroring helper, and rs.GetCreatedAt still falls back to the trait for older data. No go.mod/go.sum, vendor, or generated-file changes, no remaining deprecated WithCreatedAt/WithStatus/With*Profile call sites in the repo, and no trait-level GetProfile()/GetStatus() reads left behind — the PR description's broader profile/status migration is a no-op here because this connector never used those options.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

  • pkg/connector/helpers.go:40 (and :59) — a nil CreatedTime becomes an explicit 1970-01-01 created_at instead of an unset field; pre-existing, but worth guarding while the call is being rewritten.
Prompt for AI agents
Verify each finding against the current code and only fix it if needed.

## Suggestions

In `pkg/connector/helpers.go`:
- Around line 36-40 (protoUserToResource) and line 56-59 (protoServiceAccountToResource):
  `rs.WithResourceCreatedAt(proto.GetCreatedTime().AsTime())` is called unconditionally.
  When the upstream proto has no `created_time`, `GetCreatedTime()` returns nil and
  `AsTime()` yields the Unix epoch (1970-01-01), so the synced resource carries an
  explicit but incorrect `created_at` and `HasCreatedAt()` reports true. Fix by building
  the resource-option slice conditionally, e.g. start with
  `opts := []rs.ResourceOption{rs.WithAnnotation(annos)}` (omit the annotation for the
  service-account case) and only `opts = append(opts, rs.WithResourceCreatedAt(
  proto.GetCreatedTime().AsTime()))` when `proto.GetCreatedTime() != nil`, then pass
  `opts...` to `rs.NewUserResource`. This is pre-existing behavior inherited from the
  deprecated `rs.WithCreatedAt` call, not a regression introduced by this PR.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No blocking issues found.

@laurenleach
laurenleach merged commit 5b41b5b into main Aug 14, 2026
13 checks passed
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.

1 participant