feat: support passing multiple trustedRepositories #41
Conversation
|
SonarCloud Quality Gate failed.
|
| readonly trustedRepositories?: TrustedRepository[]; | ||
| } | ||
|
|
||
| export interface TrustedRepository { |
There was a problem hiding this comment.
TrustedRepository takes the same top-level owner/repo and optionally filter props.
| * | ||
| * @default - required if top-level owner/repo/filter not set | ||
| */ | ||
| readonly trustedRepositories?: TrustedRepository[]; |
There was a problem hiding this comment.
Up for discussion - would it be clearer to deprecate / remove the top-level properties and force people to use trustedRepostories instead?
|
|
||
| if (!trustedRepositoriesSet && (props.owner === undefined || props.repo === undefined)) { | ||
| cdk.Annotations.of(scope).addError("If you don't provide `trustedRepositories`, you must provide `owner` and `repo`."); | ||
| } |
There was a problem hiding this comment.
If we decided to deprecate the top-level properties, we could annotate the deprecated properties here.
| const roleProps = GithubActionsRole.extractRoleProps(props); | ||
|
|
||
| // The actual IAM Role creation | ||
| super(scope, id, { | ||
| ...roleProps, | ||
| assumedBy: new iam.WebIdentityPrincipal(provider.openIdConnectProviderArn, { | ||
| StringLike: { | ||
| 'ForAnyValue:StringLike': { |
There was a problem hiding this comment.
This should be safe even if just one property is passed. From the docs:
ForAnyValue – This qualifier tests whether at least one member of the set of request context key values matches at least one member of the set of context key values in your policy condition. The context key returns true if any one of the context key values in the request matches any one of the context key values in the policy. For no matching context key or a null dataset, the condition returns false.
If we think people might be worried about seeing a cdk diff with this, we could optionally use ForAnyValue only when multiple repos are passed.
|
Quality gate seems to be complaining about the tests having duplication... |
|
Hey @aripalo - any chance we can get this reviewed? If this isn't the direction you want to go with this repo, I might fork and publish a separate package. TYIA for your time and this great project! |
This PR introduces a new API that allows passing multiple subjects via the
trustedRepositoriesproperty.Fixes #35