Upbound CrossPlane addon upgrade #1029
Conversation
elamaran11
requested review from
aws-ia-ci,
parkand1,
shapirov103,
youngjeong46 and
a team
as code owners
|
/do-e2e-tests |
|
@shapirov103 Please review and merge this. Lets get this to release. |
| import {createNamespace, supportsALL} from '../../utils'; | ||
| import { Policy, PolicyDocument} from 'aws-cdk-lib/aws-iam'; | ||
| import * as cdk from 'aws-cdk-lib'; | ||
| import {HelmAddOn, HelmAddOnProps, HelmAddOnUserProps} from "../helm-addon"; |
| /* | ||
| * EKS MasterRole. `mastersRole` of blueprints should be passed to this parameter. | ||
| */ | ||
| eksMasterRole?: any; |
There was a problem hiding this comment.
Why does it need this master role? I assume it needs role of the clusters that crossplane will manage, not the management cluster which runs crossplane.
Another question - why is type any as opposed to let's say IRole or string for arn?
There was a problem hiding this comment.
I agree and im using String which can be used on the role name.
There was a problem hiding this comment.
Can it be cross-account for real use cases? E.g. in a multi-account architecture, how do you see that role passed? We could make it IRole and allow customers either to create a role or look up role (supporting cross account).
If it is string, then no validation is applied upfront. They can pass anything, and until the attempt something later through crossplane they will see no failure.
Another option is for the string to be role arn (I can settle for this). The most generic option is role: IRole | string.
@shapirov103 All comments are fixed, please check and run e2e. |
|
@shapirov103 Please check te changes, currently doing local test with CXP. Will share details when im done testing. |
|
/do-e2e-tests |
|
/do-e2e-tests |
|
/do-e2e-tests |
|
/do-e2e-tests |
Issue #, if available:
Description of changes:
Upbound CrossPlane addon upgrade
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.