Skip to content

Conversation

kessplas
Copy link
Contributor

Issue #, if available:

Description of changes:

Previously, the CI used hard-coded AWS resource identifiers to run against, which makes running CI in a fork of the repo functionally impossible.
This change moves the CI to use the repository's Action variables to specify the resources, so that forks can configure their own AWS resources.
This does not allow execution of 3p PRs against the upstream repo when the submitting fork has not gone through the setup process.

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

Check any applicable:

  • Were any files moved? Moving files changes their URL, which breaks all hyperlinks to the files.

@kessplas kessplas requested a review from a team as a code owner June 20, 2023 20:22
@kessplas
Copy link
Contributor Author

It turns out that I missed an important detail about Github repo variables:

They are not passed to workflows that are triggered by a pull request from a fork.

Therefore, this PR does not allow for PRs from forks against upstream to run CI; no variables are used in this case, so no resources are specified and the tests cannot run. It would be possible to define our (AWS Crypto Tools owned) resources through default values (either in the yaml files somehow, or simply in the Java code) but the STS call will use the fork in the userIdentity struct and thus give AccessDenied as the role's trust policy specifies the aws org's repo.

This PR is still useful, as it allows for forks to define their own variables to specify AWS resources to test against. That at least lets the 3rd party run CI in their own fork before submitting a PR.

@kessplas
Copy link
Contributor Author

Reviewers, please reference the passing CI in my fork: https://github.com/justplaz/amazon-s3-encryption-client-java/actions/runs/5316565223

Since I am not actually a third party, I have also pushed the branch directly to upstream (this) repo: https://github.com/aws/amazon-s3-encryption-client-java/actions/runs/5327696123

josecorella
josecorella previously approved these changes Jun 22, 2023
Copy link
Contributor

@josecorella josecorella left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just some small nits, but lgtm

@kessplas
Copy link
Contributor Author

@kessplas kessplas requested a review from josecorella June 26, 2023 17:51
@kessplas kessplas merged commit 66a5ca4 into aws:main Jun 26, 2023
@kessplas kessplas deleted the fork-ci-setup branch June 26, 2023 20:50
aws-crypto-tools-ci-bot pushed a commit that referenced this pull request Aug 31, 2023
## [3.1.0](v3.0.1...v3.1.0) (2023-08-31)

### Features

* add configuration option to set max buffer size ([#166](#166)) ([ecf6e6c](ecf6e6c))
* multipart & ranged get examples ([#168](#168)) ([203e5dc](203e5dc))
* Refactor `KmsKeyring` to use `GenerateDataKey` instead of `Encrypt` ([#171](#171)) ([a1a22a4](a1a22a4))

### Fixes

* Create default wrapped clients only if necessary. ([#163](#163)) ([285eab6](285eab6))
* unwrap completion exception in AbortMultipartUpload and inside multipart putObject ([#174](#174)) ([84baad8](84baad8))

### Maintenance

* allow CI to run in forks ([#164](#164)) ([66a5ca4](66a5ca4))
* **deps-dev:** bump bcprov-jdk18on from 1.72 to 1.74 ([#169](#169)) ([5502eab](5502eab))
* fix bugs and nit ([#175](#175)) ([926818b](926818b))
* install dependabot ([#172](#172)) ([1c63fdb](1c63fdb))
* warn against use of Encryption Context for non-kms keyrings. ([#173](#173)) ([54557a9](54557a9))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants