Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Identity Center Account Assignments to Unified Resource Cache #49580

Merged
merged 3 commits into from
Dec 9, 2024

Conversation

tcsc
Copy link
Contributor

@tcsc tcsc commented Nov 29, 2024

Adds Identity Center Account Assignments to the Unified resource cache so they can be requested in access requests.

Unfortunately we can't just include an identitycenterv1.AccountAssignment directly in the resource cache ListResources output because the legacy protobuf codegen used for the authservice and the new codegen used for identitycenter/v1 produce incompatible serialization code, so resulting generated code does not compile.

To get around this issue, this change introduces a parallel (and slightly simplified) definition of an IdentityCenterAccountAssignment in the authservice protobuf spec to act as the wire format for this type. The cached identitycenterv1.AccountAssignment resources are copied into a proto.IdentityCenterAccountAssignment on a cache read.

I'm not particularly thrilled with this solution, and if anyone has an alternative that lets us use the definitions from identitycenter/v1 without this parallel definition, I'd be much obliged.

Change Includes:

  • adding resources to cache
  • adding account assignment paginated resource
  • AccountAssignment Role Condition matching for RBAC

@tcsc tcsc added no-changelog Indicates that a PR does not require a changelog entry backport/branch/v17 labels Nov 29, 2024
@tcsc tcsc requested review from r0mant and smallinsky November 29, 2024 12:36
@github-actions github-actions bot requested a review from eriktate November 29, 2024 12:37

// IdentityCenterAccountAssignment represents a requestable Identity Center
// Account Assignment
message IdentityCenterAccountAssignment {
Copy link
Contributor

@smallinsky smallinsky Dec 2, 2024

Choose a reason for hiding this comment

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

Why do we need a second duplicated IdentityCenterAccountAssignment proto definition where IdentityCenterAccountAssignment is already defined in:
https://github.com/gravitational/teleport/blob/master/api/proto/teleport/legacy/types/types.proto#L3409

Copy link
Contributor Author

@tcsc tcsc Dec 6, 2024

Choose a reason for hiding this comment

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

So, types.IdentityCenterAccountAssignment is for specifying Account Assignments in role conditions. If I had known bettrer back when defining this, I'd have called them an AccountAssignmentCondition or AccountAssignmentExpression (as they can be globbed).

This definition, proto.IdentityCenterAccountAssignment allows Account Assignment resources to be included in a list of resources returned by ListResources ( essentially by having a parallel (and slightly simplified) definition of an identitycenterv1.AccountAssignment in the legacy authservice protobuf definitions).

The primary identitycenterv1.AccountAssignment gets copied into a proto.IdentityCenterAccountAssignment at read time and then gets shipped over to whatever client invoked ListResources(), then gets re-packed back into an identitycenterv1.AccountAssignment.

The reason we can't just include an identitycenterv1.AccountAssignment in the ListResources output that the legacy protobuf codegen used for the authservice and the new codegen used for identitycenter/v1 produce incompatible serialization code, so resulting generated code does not compile.

If there is a way around this that will let us use the definitions from identitycenter/v1, I'd be much happier.

@tcsc tcsc force-pushed the tcsc/idc-expose-accounts-as-apps branch 7 times, most recently from ba6f65d to aa1132c Compare December 5, 2024 07:06
Base automatically changed from tcsc/idc-expose-accounts-as-apps to master December 5, 2024 12:43
Copy link
Collaborator

@r0mant r0mant left a comment

Choose a reason for hiding this comment

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

@tcsc Does this need a rebase? Feels like it contains code from PRs I already reviewed.

@tcsc
Copy link
Contributor Author

tcsc commented Dec 6, 2024

@tcsc Does this need a rebase? Feels like it contains code from PRs I already reviewed.

And how.

@tcsc tcsc force-pushed the tcsc/idc-expose-account-assignmens-via-resource-cache branch from a5a936b to a354833 Compare December 6, 2024 03:41
@tcsc tcsc requested a review from r0mant December 6, 2024 03:45
api/client/client.go Outdated Show resolved Hide resolved
@tcsc tcsc force-pushed the tcsc/idc-expose-account-assignmens-via-resource-cache branch from a354833 to 88f231a Compare December 6, 2024 05:01
@tcsc tcsc enabled auto-merge December 6, 2024 13:07
@tcsc tcsc requested a review from smallinsky December 6, 2024 13:37
Adds Identity Center Account Assignments to the Unified resource cache
so they can be requested in access requests.

Unfortunately we can't just include an `identitycenterv1.AccountAssignment`
directly in the resource cache ListResources output because the legacy
protobuf codegen used for the authservice and the new codegen used for
identitycenter/v1 produce incompatible serialization code, so resulting
generated code does not compile.

To get around this issue, this change introduces a parallel (and slightly
simplified) definition of an IdentityCenterAccountAssignment in the authservice
protobuf spec to act as the wire format for this type. The cached
`identitycenterv1.AccountAssignment` resources are copied into a
`proto.IdentityCenterAccountAssignment` on a cache read.

Includes:
 - adding resources to cache
 - adding account assignment paginated resource
 - account assignment role condition matching for RBAC
@tcsc tcsc force-pushed the tcsc/idc-expose-account-assignmens-via-resource-cache branch from b15b40c to a8d3470 Compare December 6, 2024 13:57
@tcsc tcsc added this pull request to the merge queue Dec 9, 2024
@public-teleport-github-review-bot public-teleport-github-review-bot bot removed the request for review from eriktate December 9, 2024 12:03
Merged via the queue into master with commit 10925bb Dec 9, 2024
43 checks passed
@tcsc tcsc deleted the tcsc/idc-expose-account-assignmens-via-resource-cache branch December 9, 2024 12:21
@public-teleport-github-review-bot

@tcsc See the table below for backport results.

Branch Result
branch/v17 Failed

tcsc added a commit that referenced this pull request Dec 10, 2024
Backports #49580

Adds Identity Center Account Assignments to the Unified resource cache
so they can be requested in access requests.

Unfortunately we can't just include an `identitycenterv1.AccountAssignment`
directly in the resource cache ListResources output because the legacy
protobuf codegen used for the authservice and the new codegen used for
identitycenter/v1 produce incompatible serialization code, so resulting
generated code does not compile.

To get around this issue, this change introduces a parallel (and slightly
simplified) definition of an IdentityCenterAccountAssignment in the authservice
protobuf spec to act as the wire format for this type. The cached
`identitycenterv1.AccountAssignment` resources are copied into a
`proto.IdentityCenterAccountAssignment` on a cache read.

Includes:
 - adding resources to cache
 - adding account assignment paginated resource
 - account assignment role condition matching for RBAC
tcsc added a commit that referenced this pull request Dec 10, 2024
Backports #49580

Adds Identity Center Account Assignments to the Unified resource cache
so they can be requested in access requests.

Unfortunately we can't just include an `identitycenterv1.AccountAssignment`
directly in the resource cache ListResources output because the legacy
protobuf codegen used for the authservice and the new codegen used for
identitycenter/v1 produce incompatible serialization code, so resulting
generated code does not compile.

To get around this issue, this change introduces a parallel (and slightly
simplified) definition of an IdentityCenterAccountAssignment in the authservice
protobuf spec to act as the wire format for this type. The cached
`identitycenterv1.AccountAssignment` resources are copied into a
`proto.IdentityCenterAccountAssignment` on a cache read.

Includes:
 - adding resources to cache
 - adding account assignment paginated resource
 - account assignment role condition matching for RBAC
tcsc added a commit that referenced this pull request Dec 10, 2024
Backports #49580 and #49977

Adds Identity Center Account Assignments to the Unified resource cache
so they can be requested in access requests.

Unfortunately we can't just include an `identitycenterv1.AccountAssignment`
directly in the resource cache ListResources output because the legacy
protobuf codegen used for the authservice and the new codegen used for
identitycenter/v1 produce incompatible serialization code, so resulting
generated code does not compile.

To get around this issue, this change introduces a parallel (and slightly
simplified) definition of an IdentityCenterAccountAssignment in the authservice
protobuf spec to act as the wire format for this type. The cached
`identitycenterv1.AccountAssignment` resources are copied into a
`proto.IdentityCenterAccountAssignment` on a cache read.

Includes:
 - adding resources to cache
 - adding account assignment paginated resource
 - account assignment role condition matching for RBAC
github-merge-queue bot pushed a commit that referenced this pull request Dec 10, 2024
…he (#49976)

Backports #49580 and #49977

Adds Identity Center Account Assignments to the Unified resource cache
so they can be requested in access requests.

Unfortunately we can't just include an `identitycenterv1.AccountAssignment`
directly in the resource cache ListResources output because the legacy
protobuf codegen used for the authservice and the new codegen used for
identitycenter/v1 produce incompatible serialization code, so resulting
generated code does not compile.

To get around this issue, this change introduces a parallel (and slightly
simplified) definition of an IdentityCenterAccountAssignment in the authservice
protobuf spec to act as the wire format for this type. The cached
`identitycenterv1.AccountAssignment` resources are copied into a
`proto.IdentityCenterAccountAssignment` on a cache read.

Includes:
 - adding resources to cache
 - adding account assignment paginated resource
 - account assignment role condition matching for RBAC
github-merge-queue bot pushed a commit that referenced this pull request Dec 10, 2024
…he (#49976)

Backports #49580 and #49977

Adds Identity Center Account Assignments to the Unified resource cache
so they can be requested in access requests.

Unfortunately we can't just include an `identitycenterv1.AccountAssignment`
directly in the resource cache ListResources output because the legacy
protobuf codegen used for the authservice and the new codegen used for
identitycenter/v1 produce incompatible serialization code, so resulting
generated code does not compile.

To get around this issue, this change introduces a parallel (and slightly
simplified) definition of an IdentityCenterAccountAssignment in the authservice
protobuf spec to act as the wire format for this type. The cached
`identitycenterv1.AccountAssignment` resources are copied into a
`proto.IdentityCenterAccountAssignment` on a cache read.

Includes:
 - adding resources to cache
 - adding account assignment paginated resource
 - account assignment role condition matching for RBAC
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aws-iam-identity-center backport/branch/v17 no-changelog Indicates that a PR does not require a changelog entry size/md
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants