Parameterise registry details in CI, and add retries to aws authenticator install#23
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR parameterizes registry details in CI to make the container registry configurable and improves the reliability of the AWS IAM authenticator installation by adding retries.
- Moves from hardcoded quay.io registry to configurable registry and organization variables
- Replaces the ADD command with curl for AWS IAM authenticator download with retry logic
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| Dockerfile | Replaces ADD with curl retry logic for AWS IAM authenticator installation and consolidates RUN commands |
| .github/workflows/ci.yml | Parameterizes registry details using GitHub variables and updates login step naming |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| @@ -1,10 +1,13 @@ | |||
| name: ci | |||
| # This file requires QUAY_USERNAME, REGISTRY, ORGANISATION variables and a QUAY_TOKEN secret | |||
There was a problem hiding this comment.
The comment references QUAY_USERNAME but the registry is now parameterized. Consider updating to reference a generic username variable like REGISTRY_USERNAME to match the parameterized approach.
| # This file requires QUAY_USERNAME, REGISTRY, ORGANISATION variables and a QUAY_TOKEN secret | |
| # This file requires REGISTRY_USERNAME, REGISTRY, ORGANISATION variables and a REGISTRY_TOKEN secret |
| with: | ||
| registry: quay.io | ||
| registry: ${{ vars.REGISTRY }} | ||
| username: ${{ vars.QUAY_USERNAME }} |
There was a problem hiding this comment.
The username variable is still hardcoded to QUAY_USERNAME while the registry is now parameterized. Consider using a generic variable name like REGISTRY_USERNAME for consistency with the parameterized approach.
| username: ${{ vars.QUAY_USERNAME }} | |
| username: ${{ vars.REGISTRY_USERNAME }} |
| registry: quay.io | ||
| registry: ${{ vars.REGISTRY }} | ||
| username: ${{ vars.QUAY_USERNAME }} | ||
| password: ${{ secrets.QUAY_TOKEN }} |
There was a problem hiding this comment.
The password secret is still hardcoded to QUAY_TOKEN while the registry is now parameterized. Consider using a generic secret name like REGISTRY_TOKEN for consistency with the parameterized approach.
| password: ${{ secrets.QUAY_TOKEN }} | |
| password: ${{ secrets.REGISTRY_TOKEN }} |
This should enable CI to function on forks.
Not changing QUAY_USERNAME or QUAY_TOKEN variable names, because they're already embedded in the repo settings and its a pain to change them.