-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat!: refactor API for simpler usage (v1.x) #11
Conversation
import { ReleasableCommits, awscdk } from "projen"; | ||
import { ProseWrap } from "projen/lib/javascript"; | ||
|
||
const project = new awscdk.AwsCdkConstructLibrary({ | ||
author: "Ben Limmer", | ||
authorAddress: "[email protected]", | ||
cdkVersion: "2.1.0", | ||
cdkVersion: "2.73.0", // Released in April 2023 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since I'm using jsii-struct-builder
to extend RoleProps
, I'm bumping to a more modern minimum CDK version. This was released one year ago, so it feels reasonable to ask people to be on at least this version when upgrading.
}); | ||
|
||
new ProjenStruct(project, { name: "RoleProps", filePath: "src/generated/IamRoleProps.ts" }).mixin( | ||
Struct.fromFqn("aws-cdk-lib.aws_iam.RoleProps").omit("assumedBy").withoutDeprecated(), | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
JSII can't handle statements like:
type MyRoleProps = Omit<RoleProps, 'assumedBy'>;
So I'm using jsii-struct-builder to accomplish the same thing.
|
||
// You can also access the role from the construct. This allows adding roles and using `grant` methods after the | ||
// construct has been created. | ||
myCircleCiRole.role.addToPolicy(new PolicyStatement({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No more role.role
access, which always felt wrong to me when using this.
|
||
If you want to use the OIDC provider in another stack, you can use the `getProviderForExport` method. | ||
The `CircleCiOidcProvider` is only created **once per account**. You can use the | ||
`CircleCiOidcProvider.fromOrganizationId` method to import a previously created provider into any stack. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is much simpler and more idiomatic to those using CDK.
@@ -0,0 +1,186 @@ | |||
# Upgrading |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you're an existing 0.x user, these docs will help you migrate!
* To create a role that can be assumed by CircleCI jobs, use the `CircleCiOidcRole` construct. | ||
*/ | ||
export class CircleCiOidcProvider extends Construct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of having a wrapping Construct
, this now just "is a" CfnOIDCProvider
for simplicity.
export class CircleCiOidcProvider extends Construct { | ||
public readonly provider: CfnOIDCProvider; | ||
export class CircleCiOidcProvider extends CfnOIDCProvider implements ICircleCiOidcProvider { | ||
public static fromOrganizationId(scope: Construct, organizationId: string): ICircleCiOidcProvider { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes it much easier and more idiomatic to import an existing provider.
} | ||
|
||
export interface CircleCiOidcRoleProps { | ||
readonly circleCiOidcProvider: CircleCiOidcProvider | ManualCircleCiOidcProviderProps; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was hacky and weird. Creating an interface is what is more common in CDK.
*/ | ||
export class CircleCiOidcRole extends Construct { | ||
readonly role: Role; | ||
export interface CircleCiOidcRoleProps extends CircleCiConfiguration, RoleProps {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's where I'm using that generated RoleProps
struct from .projenrc.ts
* | ||
* To create a role that can be assumed by CircleCI jobs, use the `CircleCiOidcRole` construct. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoops, this comment wasn't accurate before.
This PR refactors the API significantly to provide a better user experience. See
UPGRADING.md
for information on the changes and how to upgrade from 0.x.Fixes #8