-
Notifications
You must be signed in to change notification settings - Fork 97
Update e2e tests to be partitional #429
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
Conversation
Signed-off-by: Alex Richman <[email protected]>
| } | ||
|
|
||
| func getPartition(ctx context.Context, cfg aws.Config) string { | ||
| stsClient := sts.NewFromConfig(cfg) |
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 making a call to STS to obtain an ARN to parse, you can just parse the CA Arn, since you have that already at this point
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.
Yea, I was initially going to do that since that's what we do in the issuer. But, it felt weird to determine the partition from test resources that could change in the future. So this was my way of removing that overhead. I do think we should probably just do this once at the beginning tho and store it in the test context
ac31fd2 to
6feda75
Compare
| github.com/aws/aws-sdk-go-v2/service/iam v1.41.1 | ||
| github.com/aws/aws-sdk-go-v2/service/ram v1.30.3 | ||
| github.com/aws/aws-sdk-go-v2/service/sts v1.33.19 | ||
| github.com/aws/aws-sdk-go-v2 v1.39.3 |
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.
What's this update get us?
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.
adds new region endpoints
Hamidhasan
left a comment
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.
looks good, just minor comments
e2e/aws_helpers.go
Outdated
| } | ||
|
|
||
| func createCertificateAuthority(ctx context.Context, cfg aws.Config, isRSA bool) string { | ||
| func (testCtx *TestContext) createCertificateAuthority(ctx context.Context, cfg aws.Config, name string, signingAlgorithm types.SigningAlgorithm) { |
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.
do you want to update the commit message with the changes here too? thanks for updating this method
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.
will update
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.
actually removed these changes and will add them in a future PR
6feda75 to
b1b29c7
Compare
Signed-off-by: Alex Richman <[email protected]>
Signed-off-by: Alex Richman <[email protected]>
b1b29c7 to
1fe10c8
Compare
Signed-off-by: Alex Richman <[email protected]>
|
[APPROVALNOTIFIER] This PR is APPROVED Approval requirements bypassed by manually added approval. This pull-request has been approved by: The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Issue # (if applicable)
N/A
Reason for this change
Currently, the end-to-end tests only work in the AWS partition. This PR updates them to work in others as well
Description of changes
Describe any new or updated permissions being added
Description of how you validated changes