feat(iam): introduce `OidcProviderNative` construct utilizing the native CloudFormation resource by WarFox · Pull Request #28634 · aws/aws-cdk

WarFox · 2024-01-09T19:17:30Z

IAM is stable in CDK, so we should not introduce breaking changes. This PR introduces a new version of OIDC provider without introducing breaking changes.

Older iam.OpenIdConnectProvider, which uses custom resources with lambda, is marked as deprecated.

The newly introduced OidcProviderNative uses the native CloudFormation resource AWS::IAM::OIDCProvider

ThumbprintList

ThumbprintList must not be empty when using AWS::IAM::OIDCProvider
https://docs.aws.amazon.com/IAM/latest/UserGuide/id_roles_providers_create_oidc_verify-thumbprint.html
https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-iam-oidcprovider.html
https://docs.aws.amazon.com/IAM/latest/APIReference/API_CreateOpenIDConnectProvider.html

Closes #21197

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

aws-cdk-automation

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

WarFox · 2024-01-14T10:29:15Z

The integration test is failing with the following error now

Thumbprint list must contain at least one entry

 ❌ Deployment failed: Error: The stack named oidc-provider2-integ-test failed creation, it may need to be manually deleted from the AWS console: ROLLBACK_COMPLETE: Resource handler returned message: "Thumbprint list must contain at least one entry. (Service: Iam, Status Code: 400, Request ID: 117aab55-6bd3-4521-b478-c065f144c48f)" (RequestToken: 618dd5f3-0cc7-a4f0-d899-f2d793dc5f4b, HandlerErrorCode: InvalidRequest), Resource handler returned message: "Thumbprint list must contain at least one entry. (Service: Iam, Status Code: 400, Request ID: b9dbef07-5151-480e-8e74-8e15fe414db6)" (RequestToken: 35d9515c-d891-edd8-123d-73a69ffbd4af, HandlerErrorCode: InvalidRequest)

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

WarFox · 2024-01-14T14:44:02Z

Clarification Request

What do you think of renaming iam.OpenIdConnectProvider2 to iam.OidcProvider?

I chose OpenIdConnectProvider2, just to make sure not to introduce a breaking change to OpenIdConnectProvider that uses a custom resource and follows the same naming pattern.

OidcProvider will be better suited to match with the AWS::IAM::OIDCProvider resource in CloudFormation.

I suggest the following name changes:

IOpenIdConnectProvider2 - IOidcProvider
OpenIdConnectProvider2Props  -> OidcProviderProps
OpenIdConnectProvider2 -> OidcProvider

and filename change to

aws-iam/lib/oidc-provider2.ts -> aws-iam/lib/oidc-provider-cfn.ts

paulhcsun · 2024-02-22T23:18:33Z

packages/aws-cdk-lib/aws-iam/lib/oidc-provider2.ts

+    const resource = new CfnOIDCProvider(this, 'Resource', {
+      url: props.url,
+      clientIdList: props.clientIds,
+      thumbprintList: props.thumbprints,
+    });


The implementation for the original OpenIdConnectProvider passed in a CodeHash from the provider so that CFN invokes the UPDATE handler when there are code change but the properties of the resource haven't changed.

Is this problem is fixed by using CfnOIDCProvider?

For more context: https://github.com/aws/aws-cdk/pull/22802/files#r1018838729

thank you for the comment. I shall look into this

paulhcsun

Hi @WarFox, I agree that using the name OidcProvider makes sense because it better aligns with the AWS::IAM::OIDCProvider that is being used but I feel like it may create too much confusion with the old resource, at least not without a lot more documentation.

After discussing with the team I believe the best option here is to use a feature flag and add changes to the existing OpenIdConnectProvider as suggested here: #16014 (comment) with the following caveats:

The feature flag should toggle between the two constructs, OpenIdConnectProvider, and OpenIdConnectProvider2 in the constructor of OpenidConnectProvider.
Rename OpenIdConnectProvider2 to OpenIdConnectProviderNative. But don't export it, only allow it to be used via OpenIdConnectProvider + feature flag

WarFox · 2024-02-28T22:51:46Z

thanks for pointing out to #16014 (comment) @paulhcsun. I shall look into how a feature flag is helpful for this, it is interesting.

What do you think of naming it OidcProvider instead of OpenIdConnectProviderNative? When I come from using a CloudFormation resource to a CDK resource, I usually expect name parity. It will also be helpful from a search point of view i.e. someone searching for OidcProvider will find the correct resource too.

Just to confirm, is the consensus in your team NOT to deprecate OpenIdConnectProvider?

paulhcsun · 2024-02-29T18:34:50Z

Hey @WarFox,

While I agree that it would be good to have name parity with OidcProvider, I don't think the name is clear how it's different from OpenIdConnectProvider. Having Native in the name makes it explicit what the difference is. Or at the very least something like OIDCProviderNative if we want to have the resource show up when users search for "oidc".

My opinion is to go with OpenIdConnectProviderNative as it's meant to replace OpenIdConnectProvider so users switching over would expect to search for the same name when looking for this resource.

As for deprecation, we would NOT deprecate OpenIdConnectProvider as it would still be used to return the new OpenIdConnectProviderNative through its constructor when the feature flag is enabled. However we should still document somewhere that the old version which uses custom resources is deprecated. One place could be here, and then explain the new feature flag and how it would return the new version of OpenIdConnectProviderNative.

paulhcsun

Hi @WarFox, left some comments for adding validation on some of the property restrictions + integ test assertions.

paulhcsun · 2025-04-03T22:47:47Z

packages/aws-cdk-lib/aws-iam/lib/oidc-provider-native.ts

+  /**
+   * The URL of the identity provider. The URL must begin with https:// and
+   * should correspond to the iss claim in the provider's OpenID Connect ID
+   * tokens. Per the OIDC standard, path components are allowed but query
+   * parameters are not. Typically the URL consists of only a hostname, like
+   * https://server.example.org or https://example.com.
+   *
+   * You cannot register the same provider multiple times in a single AWS
+   * account. If you try to submit a URL that has already been used for an
+   * OpenID Connect provider in the AWS account, you will get an error.
+   */
+  readonly url: string;


Could we add validation in the constructor to ensure that the url beginds with https://.

And also that the minimum/maximum length is 1/255 characters respectively: https://docs.aws.amazon.com/IAM/latest/APIReference/API_CreateOpenIDConnectProvider.html

Could we also add a warning or document in the docstring that the URL cannot contain any port numbers either?

packages/aws-cdk-lib/aws-iam/lib/oidc-provider-native.ts

paulhcsun · 2025-04-03T22:49:36Z

packages/aws-cdk-lib/aws-iam/lib/oidc-provider-native.ts

+   * Typically this list includes only one entry. However, IAM lets you have up
+   * to five thumbprints for an OIDC provider. This lets you maintain multiple


Can we add validation to check that a maximum of 5 thumbprints are provided?

paulhcsun · 2025-04-03T22:50:13Z

packages/aws-cdk-lib/aws-iam/lib/oidc-provider-native.ts

+   * You must provide at least one thumbprint when creating an IAM OIDC
+   * provider. For example, assume that the OIDC provider is server.example.com


Can we add validation that fails if an empty list is passed in for this requirement?

paulhcsun · 2025-04-03T22:51:51Z

packages/@aws-cdk-testing/framework-integ/test/aws-iam/test/integ.oidc-provider-native.ts

+  value: `${minimal.oidcProviderIssuer}`,
+});
+
+new IntegTest(app, 'oidc-provider-native-integ-test', {


Can we add some assertions to verify that this works as expected when deployed?

Thanks for adding the property validations and other requested changes @WarFox! Could you also add assertions to this integ test? You can refer to this documentation on adding Assertions: https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md#integration-tests

I'll look at this during the weekend. If you could point me to any examples of integration tests that uses assertions, will help me a lot

nvm, I found these https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md#assertions

WarFox · 2025-04-24T11:08:08Z

Hi @WarFox, left some comments for adding validation on some of the property restrictions + integ test assertions.

Thanks @paulhcsun, I have carved out some time during this weekend to spend on this

Pull request has been modified.

paulhcsun · 2025-05-02T22:30:10Z

packages/aws-cdk-lib/aws-iam/lib/oidc-provider-native.ts

+   * Obtain the thumbprint of the root certificate authority from the provider's
+   * server as described in https://docs.aws.amazon.com/IAM/latest/UserGuide/id_roles_providers_create_oidc_verify-thumbprint.html
+   */
+  readonly thumbprints: string[];


I don't think this property should be required. From the CFN docs it seems like it's optional and that there is a default certificate used if none is provided:

This property is optional. If it is not included, IAM will retrieve and use the top intermediate certificate authority (CA) thumbprint of the OpenID Connect identity provider server certificate.

I've checked with the service team and they said that if customer does not provide thumbprint, IAM will query Discovery Service to see if the Url provided is a valid Url, and based on the response IAM will either reject or accept the Url that customer provided.

I think we should make this optional and document this behaviour in the docstring and follow the service team's behaviour where possible.

have made it optional

WarFox · 2025-05-28T17:49:04Z

thanks for pushing this and updating the snapshots @paulhcsun

It wasn't working for me, probably because of my broken local setup

paulhcsun · 2025-05-28T18:15:52Z

All good! Thanks for adding the integration test assertions :) I totally missed it before when I was just checking the commit messages. Once the build passes I'll be happy to approve this and get this finally merged in. Thank you so much for your patience and work put into this contribution!

As a note for any local build failures or general issues with unknown causes, I usually just run git clean -fqdx . and then rebuild as if I cloned a new CDK repo and that usually fixes things up!

mergify · 2025-05-28T18:20:17Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

aws-cdk-automation · 2025-05-28T18:51:22Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
Commit ID: b781801
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

mergify · 2025-05-28T18:52:02Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

github-actions · 2025-05-28T18:52:19Z

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

github-actions bot added the beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK label Jan 9, 2024

aws-cdk-automation requested a review from a team January 9, 2024 19:17

github-actions bot added effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p1 labels Jan 9, 2024

aws-cdk-automation previously requested changes Jan 9, 2024

View reviewed changes

WarFox changed the title ~~Introduce OpenIdConnectProvider2 with native CloudFormation resource~~ fix: introduce OpenIdConnectProvider2 with native CloudFormation resource Jan 9, 2024

WarFox force-pushed the 21197-simplify-openidconnect-provider branch 4 times, most recently from 3036ef0 to 02274f7 Compare January 13, 2024 23:42

aws-cdk-automation added the pr/reviewer-clarification-requested The contributor has requested clarification on feedback, a failing build, or a failing PR Linter run label Jan 14, 2024

WarFox force-pushed the 21197-simplify-openidconnect-provider branch from f9af6d6 to 35e95ea Compare January 14, 2024 14:59

WarFox marked this pull request as ready for review January 14, 2024 16:09

WarFox force-pushed the 21197-simplify-openidconnect-provider branch from 35e95ea to a4a4a18 Compare January 19, 2024 22:34

aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Jan 19, 2024

WarFox force-pushed the 21197-simplify-openidconnect-provider branch from a4a4a18 to 8f7bc78 Compare January 21, 2024 21:20

WarFox mentioned this pull request Jan 21, 2024

(aws-iam): (Simplify OpenIdConnectProvider by using CloudFormation resource instead of custom resource lambda) #21197

Closed

2 tasks

github-actions bot mentioned this pull request Feb 1, 2024

Monthly PR metrics report - Jan 2024 #28952

Closed

WarFox force-pushed the 21197-simplify-openidconnect-provider branch from 8f7bc78 to 211248a Compare February 6, 2024 10:15

paulhcsun changed the title ~~fix: introduce OpenIdConnectProvider2 with native CloudFormation resource~~ fix(iam): introduce OpenIdConnectProvider2 with native CloudFormation resource Feb 22, 2024

paulhcsun reviewed Feb 22, 2024

View reviewed changes

paulhcsun previously requested changes Feb 22, 2024

View reviewed changes

aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Feb 22, 2024

paulhcsun removed the pr/reviewer-clarification-requested The contributor has requested clarification on feedback, a failing build, or a failing PR Linter run label Feb 23, 2024

paulhcsun previously requested changes Apr 3, 2025

View reviewed changes

aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Apr 11, 2025

Merge branch 'main' into 21197-simplify-openidconnect-provider

7653293

aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Apr 24, 2025

paulhcsun reviewed May 2, 2025

View reviewed changes

WarFox and others added 15 commits May 7, 2025 12:06

Merge branch 'main' into 21197-simplify-openidconnect-provider

23790f6

add validation for OidcProviderNative properties

159f2b5

thumbprints in iam/oidc-provider-native is optional

0bd8093

fix eslint erro

7d49a8a

add @default documentation for thumbprints

bf4717e

no trailing spaces allowed

4b01f6e

add tests for OidcProviderNative property validations

0847e1b

Merge branch 'main' into 21197-simplify-openidconnect-provider

4658f67

maximum allowed length of url is 255

add88e9

Merge branch 'main' into 21197-simplify-openidconnect-provider

b46e335

ci: add assertions in integration tests for oidc-provider-native

e3e2ab3

Merge branch 'main' into 21197-simplify-openidconnect-provider

85b80f2

Merge branch 'main' into 21197-simplify-openidconnect-provider

c915a9f

Merge branch 'main' into 21197-simplify-openidconnect-provider

a7f18a5

update snapshots

e04a40a

paulhcsun approved these changes May 28, 2025

View reviewed changes

Merge branch 'main' into 21197-simplify-openidconnect-provider

b781801

		* Typically this list includes only one entry. However, IAM lets you have up
		* to five thumbprints for an OIDC provider. This lets you maintain multiple

		* You must provide at least one thumbprint when creating an IAM OIDC
		* provider. For example, assume that the OIDC provider is server.example.com

Conversation

WarFox commented Jan 9, 2024 • edited by paulhcsun Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ThumbprintList

Uh oh!

aws-cdk-automation left a comment

Choose a reason for hiding this comment

Uh oh!

WarFox commented Jan 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

WarFox commented Jan 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

paulhcsun left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

WarFox commented Feb 28, 2024

Uh oh!

paulhcsun commented Feb 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

paulhcsun left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

WarFox commented Apr 24, 2025

Uh oh!

paulhcsun May 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

WarFox commented May 28, 2025

Uh oh!

paulhcsun commented May 28, 2025

Uh oh!

mergify bot commented May 28, 2025

Uh oh!

aws-cdk-automation commented May 28, 2025

AWS CodeBuild CI Report

Uh oh!

mergify bot commented May 28, 2025

Uh oh!

github-actions bot commented May 28, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

WarFox commented Jan 9, 2024 •

edited by paulhcsun

Loading

WarFox commented Jan 14, 2024 •

edited

Loading

WarFox commented Jan 14, 2024 •

edited

Loading

paulhcsun left a comment •

edited

Loading

paulhcsun commented Feb 29, 2024 •

edited

Loading

paulhcsun May 2, 2025 •

edited

Loading