-
Notifications
You must be signed in to change notification settings - Fork 3
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
feat: add release workflow for kustomize oci image #1379
base: main
Are you sure you want to change the base?
Conversation
chore: add release pipeline for dis-apim kustomize resources
Important Review skippedReview was skipped as selected files did not have any reviewable changes. 💤 Files selected but had no reviewable changes (1)
You can disable this status message by setting the 📝 WalkthroughWalkthroughA new GitHub Actions workflow file named Changes
Sequence Diagram(s)Latest Job WorkflowsequenceDiagram
participant User as Committer
participant GH as GitHub Actions
participant Repo as Repository
participant Flux as Flux Config
participant Reg as Container Registry
User->>GH: Push commit to main branch
GH->>Repo: Checkout repository
GH->>Flux: Configure Flux and set variables
GH->>Reg: Login using GitHub Token
GH->>Reg: Push artifact (tag: latest)
Release Job WorkflowsequenceDiagram
participant Tagger as Tagger
participant GH as GitHub Actions
participant Repo as Repository
participant Flux as Flux Config
participant Reg as Container Registry
Tagger->>GH: Push tag matching kustomize-dis-apim-*
GH->>Repo: Checkout repository
GH->>Flux: Configure Flux and set variables
GH->>GH: Extract tag from reference
GH->>Reg: Login using GitHub Token
GH->>Reg: Push artifact (tag: extracted tag)
Suggested reviewers
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (13)
services/dis-apim-operator/config/default/kustomization.yaml (4)
9-10
: Remove trailing spaces.
A trailing space was detected on line 10. Please remove it to maintain a clean YAML file.
21-22
: Review comment on commented block indentation.
Static analysis flagged indentation issues in the commented lines (e.g. lines 21–22). Although these are comments, consider cleaning up their indentation to improve readability.
50-112
: Validate Certificate Replacement Configuration.
The replacements block for certificate injection into both Validating and Mutating webhook configurations is now active. Please double-check that the indices (e.g. use of index 0 and index 1) correctly map to the intended DNS names and injection targets, and that the Certificate resource namedserving-cert
exists and is referenced properly.
144-178
: Review Service Replacement Configuration.
This block maps fields from thewebhook-service
to Certificate resources via two separate list items. Confirm that the service’sname
andnamespace
fields are correctly being replaced—especially the use of delimiters and indices for extracting DNS names..github/workflows/dis-apim-kustomize-realease.yaml (9)
10-10
: Remove trailing spaces.
A trailing space was detected at the end of line 10. Please remove it to maintain consistency and avoid linting errors.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 10-10: trailing spaces
(trailing-spaces)
21-21
: Fix job key indentation.
Thelatest:
job (line 21) is indented with 4 spaces but should be indented with 2 spaces relative to its parent (jobs:
). Adjusting this will align with GitHub Actions YAML style guidelines.🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 21-21: wrong indentation: expected 2 but found 4
(indentation)
22-22
: Adjust nested indentation in job configuration.
The nested keys under thelatest:
job (line 22) currently use 8 spaces where 6 are expected. Please update these indentations for consistency throughout the workflow file.🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 22-22: wrong indentation: expected 6 but found 8
(indentation)
25-25
: Fix defaults block indentation.
Within thelatest:
job, therun:
key underdefaults:
(line 25) is indented with 12 spaces instead of the expected 10. Correcting this ensures clarity and YAML compliance.🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 25-25: wrong indentation: expected 10 but found 12
(indentation)
26-26
: Correct working-directory indentation.
Theworking-directory
setting (line 26) is indented with 16 spaces, though 14 are expected. Please adjust this to maintain proper structure.🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 26-26: wrong indentation: expected 14 but found 16
(indentation)
54-54
: Correct indentation in release job.
For therelease:
job, thename:
field on line 54 appears indented with 8 spaces instead of the expected 6. Please re-indent this section to meet YAML formatting standards.🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 54-54: wrong indentation: expected 6 but found 8
(indentation)
57-57
: Fix defaults block indentation in release job.
Thedefaults:
block in therelease
job (line 57) is indented with 12 spaces rather than 10. Proper indentation is important to avoid YAML parsing issues.🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 57-57: wrong indentation: expected 10 but found 12
(indentation)
58-58
: Adjust working-directory indentation in release job.
Theworking-directory
key under therelease:
job is indented with 16 spaces on line 58, but 14 spaces are expected. Please make the necessary adjustment.🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 58-58: wrong indentation: expected 14 but found 16
(indentation)
86-86
: Add newline at end of file.
The file is missing a newline character at the end. Kindly add one to comply with standard file formatting conventions.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 86-86: no new line character at the end of file
(new-line-at-end-of-file)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/dis-apim-kustomize-realease.yaml
(1 hunks)services/dis-apim-operator/config/default/kustomization.yaml
(3 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/dis-apim-kustomize-realease.yaml
[error] 10-10: trailing spaces
(trailing-spaces)
[warning] 21-21: wrong indentation: expected 2 but found 4
(indentation)
[warning] 22-22: wrong indentation: expected 6 but found 8
(indentation)
[warning] 25-25: wrong indentation: expected 10 but found 12
(indentation)
[warning] 26-26: wrong indentation: expected 14 but found 16
(indentation)
[warning] 54-54: wrong indentation: expected 6 but found 8
(indentation)
[warning] 57-57: wrong indentation: expected 10 but found 12
(indentation)
[warning] 58-58: wrong indentation: expected 14 but found 16
(indentation)
[error] 86-86: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Run on Ubuntu
- GitHub Check: Run on Ubuntu
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
🔇 Additional comments (1)
services/dis-apim-operator/config/default/kustomization.yaml (1)
24-25
: Enable cert-manager resource.
The cert-manager resource (line 25) has now been uncommented. Please verify that the referenced cert-manager configuration is correct and that all related files (like certificate definitions) are aligned with this change.
- name: Setup vars | ||
id: vars | ||
run: | | ||
tag=${GITHUB_REF/refs\/tags\/${{ env.ARTIFACT_NAME }}-/} |
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.
Hmm, what are the \
used for here?
I thought it was to escape the forward slashes but then I noticed that some of them are not being escaped.
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.
They are for escaping the forwardslashes, but not the onces used by shell parameter expansion
example command to run:
export REF=refs/tags/prefix-1.0.0
echo ${REF/refs\/tags\/prefix-/}
This feels like it should be two different PRs 😅 |
Yeah 😅, I got lazy 🙈 |
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.
Actionable comments posted: 5
🧹 Nitpick comments (3)
.github/workflows/dis-apim-kustomize-realease.yml (3)
35-38
: Pin Flux Action Version for Stability
The workflow uses the Flux action with:uses: fluxcd/flux2/action@3b42b200d376430f0e24d35f1a600447d92da531 with: version: latestUsing
latest
or a commit hash could lead to non-deterministic builds if upstream changes occur. It is recommended to pin the action to a specific, released version to ensure build reproducibility.
44-49
: Use GitHub Context Variables for Revision Information (Latest Job)
The current script retrieves commit details using shell commands likegit rev-parse --short HEAD
andgit branch --show-current
. For better clarity and resilience (especially in detached HEAD states), consider using GitHub Actions’ built-in context variables such as${{ github.sha }}
for the commit hash and${{ github.ref_name }}
for the branch name.
78-83
: Use GitHub Context Variables for Revision Information (Release Job)
In the release job, the script employs git commands (git rev-parse --short HEAD
andgit branch --show-current
) to fetch revision details. For consistency and enhanced reliability—especially during tag events where branch information might not be available—consider using context variables like${{ github.sha }}
and${{ github.ref_name }}
provided by GitHub Actions.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/dis-apim-kustomize-realease.yml
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Run on Ubuntu
- GitHub Check: Run on Ubuntu
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
Split the change into two PRs. kustomization changes are moved to #1392 |
echo ${{ secrets.GITHUB_TOKEN }} | docker login ghcr.io -u ${{ github.actor }} --password-stdin | ||
container_registry=ghcr.io/${{ steps.vars.outputs.reponame }} | ||
artifact_name=${{ env.ARTIFACT_NAME }} | ||
flux push artifact oci://${container_registry}/${artifact_name}:$(git rev-parse --short HEAD) \ |
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.
Not sure I follow what is happening here.
How are you planning to integrate with: https://github.com/Altinn/altinn-platform/blob/main/services/dis-apim-operator/Makefile#L131-L135?
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.
I take all the kustomization and other files in the config folder and backage them up in a OCI image that is pushed to ghcr.
Frankly I wasn't planning on implementing it. I plan to deploy this with flux Kustomization CRDs and that resource reads kustomization.yaml and creates the final result by it self. Then we aren't relying on having a extra step in this pipeline
chore: add release pipeline for dis-apim kustomize resources
Description
Enable cert-manager integration
As the operator has a DefaultingWebhook it relies on a certificate, we are installing cert-manager so we can leverage it to generate it for us
Release pipeline for kustomize
Pipeline for building and releasing kustomize resources for dis-apim in an oci image
Related Issue(s)
Verification
Documentation
Summary by CodeRabbit