ci: add GHA #55
ci: add GHA #55
Conversation
📝 WalkthroughWalkthroughAdds a reusable GitHub Actions workflow at .github/workflows/common-helmfile.yml that accepts inputs Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| jobs: | ||
| run-helmfile: | ||
| name: run-helmfile | ||
| runs-on: ["self-hosted"] |
verbotenj
force-pushed
the
feat/add-gha
branch
2 times, most recently
from
e81f18b to
fd62585
Compare
verbotenj
force-pushed
the
feat/add-gha
branch
from
fd62585 to
e2e0984
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
helmfile-app/grafana-alloy/values.yaml.gotmpl (1)
167-175: Consider moving the Grafana username to encrypted secrets.While the password is properly templated from encrypted secrets (
{{ .Values.grafana.password }}), the username is hardcoded as"1834751". For consistency and better secret management, consider moving the username tohelmfile-app/vars/secrets.yamland templating it similarly to the password.Apply this diff to improve secret management:
stringData: - GRAFANA_USERNAME: "1834751" + GRAFANA_USERNAME: "{{ .Values.grafana.username }}" GRAFANA_PASSWORD: "{{ .Values.grafana.password }}"Then add the username to
helmfile-app/vars/secrets.yaml:grafana: username: ENC[...] password: ENC[...]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/workflows/common-helmfile.yml(1 hunks).github/workflows/pr-helmfile.yml(1 hunks).github/workflows/run-helmfile.yml(1 hunks).github/workflows/test-helmfile.yml(1 hunks).github/workflows/test-terraform.yml(1 hunks)helmfile-app/grafana-alloy/values.yaml.gotmpl(3 hunks)helmfile-app/vars/secrets.yaml(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-28T22:08:40.542Z
Learnt from: wolf31o2
Repo: blinklabs-io/vpn-infrastructure PR: 51
File: .github/workflows/conventional-commits.yml:16-17
Timestamp: 2025-10-28T22:08:40.542Z
Learning: The webiny/action-conventional-commits GitHub Action reports commit message validation by exiting with a non-zero exit code (failing the job) rather than posting status updates via the GitHub API. Therefore, it only requires `contents: read` permission, not `checks: write` or `pull-requests: write`.
Applied to files:
.github/workflows/run-helmfile.yml.github/workflows/pr-helmfile.yml.github/workflows/test-helmfile.yml.github/workflows/common-helmfile.yml
🔇 Additional comments (11)
helmfile-app/vars/secrets.yaml (1)
1-60: LGTM! Secrets properly encrypted with SOPS.The grafana.password ciphertext has been updated along with the SOPS metadata (lastmodified and MAC), which is consistent with credential rotation. All secrets remain properly encrypted using SOPS with AES256_GCM and AWS KMS, ensuring no plaintext credentials are exposed in the repository.
.github/workflows/test-terraform.yml (1)
6-8: LGTM! Formatting improvement for readability.The YAML formatting has been updated to use multi-line list syntax, which improves readability without changing the workflow's behavior.
helmfile-app/grafana-alloy/values.yaml.gotmpl (2)
47-49: LGTM! Credentials now sourced from environment variables.Replacing hardcoded credentials with environment variable lookups (
env("GRAFANA_USERNAME")andenv("GRAFANA_PASSWORD")) improves security by enabling secret-based credential management.
155-165: LGTM! Proper injection of credentials from Kubernetes Secret.The
extraEnvblock correctly injects Grafana credentials from thegrafana-cloudSecret into the pod environment, ensuring credentials are not hardcoded in the configuration..github/workflows/pr-helmfile.yml (1)
1-21: LGTM! Well-structured workflow with proper permissions.The workflow is well-designed with:
- Minimal permissions (
contents: read)- Path-based triggers to run only when relevant files change
- Reusable workflow pattern for consistency
- Matrix strategy for future extensibility
.github/workflows/run-helmfile.yml (1)
1-26: LGTM! Well-designed manual workflow with proper security controls.The workflow is properly structured with:
- Minimal permissions (
contents: read)- Clear input definitions with descriptions and defaults
- Type-safe choice input for cluster selection
- Reusable workflow pattern for consistency
- Secure secret inheritance
.github/workflows/test-helmfile.yml (1)
1-23: LGTM! Test workflow with proper security controls.The workflow is well-structured with:
- Minimal permissions (
contents: read)- Clear cluster input definition
- Hardcoded
"diff"argument appropriate for testing- Reusable workflow pattern for consistency
.github/workflows/common-helmfile.yml (4)
1-67: LGTM! Well-secured reusable workflow with excellent security practices.The workflow demonstrates strong security posture:
- Minimal permissions (
contents: read)- SHA-pinned action versions for supply chain security
- Conditional AWS configuration based on cluster input
- Secrets properly sourced from GitHub Secrets
- Clean separation of concerns with reusable pattern
17-17: Clarify runner choice: ubuntu-latest vs self-hosted.The workflow uses
ubuntu-latestas the runner, but a past review comment from @agaffney asked "Why self-hosted?" This suggests there may have been an expectation or requirement to use self-hosted runners. Please clarify:
- Is
ubuntu-latest(GitHub-hosted) acceptable for this workflow?- Are there networking, access, or compliance requirements that would necessitate self-hosted runners?
- Do the AWS credentials and EKS cluster access work correctly from GitHub-hosted runners?
35-51: AWS CLI installation script is well-written.The installation script properly:
- Checks if AWS CLI already exists to avoid redundant installs
- Downloads from the official AWS source
- Installs to user directory to avoid permission issues
- Updates
$GITHUB_PATHcorrectly- Cleans up temporary files
66-66: Verification Complete: Helmfile environment configuration is correct.The Helmfile configuration in
helmfile-app/helmfile-defaults.yaml.gotmpldefines environments that match cluster names (e.g.,aws-vpn,local-vpn,default). The workflow's use of-e ${{ inputs.cluster }}is the correct Helmfile syntax for selecting the target environment. No issues found.
.github/workflows/pr-helmfile.yml
Outdated
| paths: | ||
| - "helmfile-app/**" | ||
| - ".github/workflows/test-helmfile.yml" |
There was a problem hiding this comment.
🧩 Analysis chain
Verify the path filter reference.
Line 6 references .github/workflows/test-helmfile.yml, but this workflow file is pr-helmfile.yml. Did you intend to reference .github/workflows/pr-helmfile.yml instead? If this workflow should trigger when test-helmfile.yml changes, please clarify the reasoning.
Run the following script to check the relationship between these workflow files:
🏁 Script executed:
#!/bin/bash
# Check if test-helmfile.yml references pr-helmfile.yml or if this is a typo
echo "=== Checking pr-helmfile.yml paths trigger ==="
grep -n "paths:" .github/workflows/pr-helmfile.yml -A5
echo -e "\n=== Checking if test-helmfile.yml and pr-helmfile.yml share common patterns ==="
if [ -f .github/workflows/test-helmfile.yml ]; then
echo "test-helmfile.yml exists"
grep -n "uses:" .github/workflows/test-helmfile.yml
grep -n "uses:" .github/workflows/pr-helmfile.yml
else
echo "test-helmfile.yml not found"
fiLength of output: 384
I need to examine the full content and purpose of both workflow files to understand if this cross-reference is intentional.
---
🏁 Script executed:
```shell
#!/bin/bash
# Get full content of both workflow files to understand their purposes
echo "=== Full pr-helmfile.yml ==="
cat .github/workflows/pr-helmfile.yml
echo -e "\n=== Full test-helmfile.yml ==="
cat .github/workflows/test-helmfile.yml
Length of output: 864
Update line 6 to reference .github/workflows/pr-helmfile.yml instead of test-helmfile.yml.
The current path trigger references test-helmfile.yml, which is a manual-only workflow (triggered via workflow_dispatch). The pr-helmfile.yml workflow should retrigger when changes are made to itself, not to the separate manual workflow. Update the path filter to reference .github/workflows/pr-helmfile.yml.
🤖 Prompt for AI Agents
.github/workflows/pr-helmfile.yml lines 4-6: the path filter currently
references ".github/workflows/test-helmfile.yml" but should reference the
workflow file itself so changes to pr-helmfile.yml retrigger the workflow; edit
line 6 to change the path to ".github/workflows/pr-helmfile.yml" (replace
test-helmfile.yml with pr-helmfile.yml) so the workflow is triggered on edits to
its own file.
949bae4 to
0225f87
Compare
verbotenj
force-pushed
the
feat/add-gha
branch
from
0225f87 to
5c4a696
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/common-helmfile.yml (1)
38-54: Consider using an established AWS CLI installation action.The custom bash script for installing AWS CLI v2 works, but using a maintained action (e.g.,
unfor19/install-aws-cli-action) would reduce maintenance burden and improve consistency across workflows. The current approach is functional and safe, but worth considering as a refactor.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/workflows/common-helmfile.yml(1 hunks).github/workflows/pr-helmfile.yml(1 hunks).github/workflows/run-helmfile.yml(1 hunks).github/workflows/test-helmfile.yml(1 hunks).github/workflows/test-terraform.yml(1 hunks)helmfile-app/grafana-alloy/values.yaml.gotmpl(3 hunks)helmfile-app/vars/secrets.yaml(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- .github/workflows/run-helmfile.yml
- .github/workflows/test-helmfile.yml
- helmfile-app/vars/secrets.yaml
- .github/workflows/pr-helmfile.yml
- .github/workflows/test-terraform.yml
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-28T22:08:40.542Z
Learnt from: wolf31o2
Repo: blinklabs-io/vpn-infrastructure PR: 51
File: .github/workflows/conventional-commits.yml:16-17
Timestamp: 2025-10-28T22:08:40.542Z
Learning: The webiny/action-conventional-commits GitHub Action reports commit message validation by exiting with a non-zero exit code (failing the job) rather than posting status updates via the GitHub API. Therefore, it only requires `contents: read` permission, not `checks: write` or `pull-requests: write`.
Applied to files:
.github/workflows/common-helmfile.yml
🔇 Additional comments (6)
.github/workflows/common-helmfile.yml (4)
11-12: Permissions block properly configured. ✓The
permissions: contents: readis appropriate for this reusable workflow and aligns with the learnings on minimal permission scoping. This addresses the prior security finding.
19-29: All third-party actions are correctly pinned to commit hashes. ✓Every action uses a full commit SHA, not just a semantic version tag, which prevents unexpected behavior from upstream changes. This addresses the prior finding regarding the mozilla-sops-action.
30-36: AWS credentials properly sourced from GitHub Secrets. ✓The workflow uses
${{ secrets.VPN_OPS_AWS_ACCESS_KEY_ID }}and${{ secrets.VPN_OPS_AWS_SECRET_ACCESS_KEY }}, which prevents credential exposure in logs and aligns with the PR objective of storing sensitive data only in Secret resources.
60-70: No action required—helmfile workspace is properly configured.The helmfile-app directory contains all necessary dependencies and configuration files. The main configuration exists as
helmfile.yaml.gotmpl(a Go template form), which correctly includeshelmfile-defaults.yaml.gotmplandhelmfile-repos.yaml. The.sops.yamlfile is present with AWS KMS encryption configured for the helm-secrets plugin. Variables and secrets are pre-loaded via the environments configuration. Thehelmfile-auto-init: "false"setting is appropriate and correctly aligned with this pre-configured environment—no automatic initialization is needed.helmfile-app/grafana-alloy/values.yaml.gotmpl (2)
47-48: LGTM! Environment variable pattern is secure.Using
env()to reference credentials prevents them from being logged or exposed in the configuration. This aligns well with the PR objective of keeping secrets out of workflow logs.
155-165: LGTM! Proper Kubernetes Secret injection pattern.The
extraEnvconfiguration correctly injects credentials from thegrafana-cloudSecret as environment variables, following Kubernetes best practices.
verbotenj
force-pushed
the
feat/add-gha
branch
from
5c4a696 to
b429c22
Compare
Signed-off-by: Ales Verbic <[email protected]>
verbotenj
force-pushed
the
feat/add-gha
branch
from
b429c22 to
9e5cbd2
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
helmfile-app/grafana-alloy/values.yaml.gotmpl (1)
167-175: Move hardcoded username to SOPS-encrypted secrets file.Line 174 contains the Grafana username
"1834751"in plaintext, which violates the PR objective: "Ensure no secrets are exposed in repository files or workflow logs (nothing secret outside Secret resources)."While the password correctly uses a template from the SOPS-encrypted
secrets.yaml, the username should follow the same pattern for consistency and defense-in-depth.Apply this diff to reference the username from encrypted values:
stringData: - GRAFANA_USERNAME: "1834751" + GRAFANA_USERNAME: "{{ .Values.grafana.username }}" GRAFANA_PASSWORD: "{{ .Values.grafana.password }}"Then add
grafana.usernameto the SOPS-encryptedhelmfile-app/vars/secrets.yamlfile alongside the existinggrafana.password.
🧹 Nitpick comments (1)
.github/workflows/common-helmfile.yml (1)
19-28: Fix typo in kubectl setup comment.All actions are properly pinned to commit hashes, which is excellent for security. However, there's a minor typo in the version comment.
Apply this diff to fix the typo:
- name: Install kubectl - uses: azure/setup-kubectl@0c5e050edfed71b2b50731ab044d42489d51c129 # vv4.0.0 https://github.com/Azure/setup-kubectl/releases/tag/vv4.0.0 + uses: azure/setup-kubectl@0c5e050edfed71b2b50731ab044d42489d51c129 # v4.0.0 https://github.com/Azure/setup-kubectl/releases/tag/v4.0.0
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/workflows/common-helmfile.yml(1 hunks).github/workflows/pr-helmfile.yml(1 hunks).github/workflows/run-helmfile.yml(1 hunks).github/workflows/test-helmfile.yml(1 hunks).github/workflows/test-terraform.yml(1 hunks)helmfile-app/grafana-alloy/values.yaml.gotmpl(3 hunks)helmfile-app/vars/secrets.yaml(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- .github/workflows/test-helmfile.yml
- .github/workflows/run-helmfile.yml
- .github/workflows/pr-helmfile.yml
- helmfile-app/vars/secrets.yaml
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-28T22:08:40.542Z
Learnt from: wolf31o2
Repo: blinklabs-io/vpn-infrastructure PR: 51
File: .github/workflows/conventional-commits.yml:16-17
Timestamp: 2025-10-28T22:08:40.542Z
Learning: The webiny/action-conventional-commits GitHub Action reports commit message validation by exiting with a non-zero exit code (failing the job) rather than posting status updates via the GitHub API. Therefore, it only requires `contents: read` permission, not `checks: write` or `pull-requests: write`.
Applied to files:
.github/workflows/common-helmfile.yml
🔇 Additional comments (8)
.github/workflows/test-terraform.yml (1)
6-8: LGTM: Formatting update.The trigger formatting changes are cosmetic and semantically equivalent to the previous version.
helmfile-app/grafana-alloy/values.yaml.gotmpl (2)
47-48: LGTM: Proper credential referencing.Using
env()to read credentials from environment variables is the correct approach for Grafana Alloy configuration.
155-165: LGTM: Proper environment variable injection.The
extraEnvconfiguration correctly injects environment variables from thegrafana-cloudSecret using Kubernetes-nativesecretKeyRef..github/workflows/common-helmfile.yml (5)
1-13: LGTM: Proper reusable workflow setup.The workflow is correctly configured as reusable with
workflow_call, required inputs, and minimal permissions (contents: read). This addresses the previous code scanning alert about missing permissions.
30-46: LGTM: Secure AWS credentials handling.AWS credentials are correctly sourced from GitHub Secrets and conditionally configured only for the
aws-vpncluster. The use of heredoc syntax prevents injection issues.
47-64: LGTM: Robust AWS CLI installation.The installation script correctly:
- Checks if AWS CLI is already available before installing
- Uses the official AWS CLI v2 installer
- Installs to a local directory (appropriate for GitHub Actions)
- Handles PATH manipulation for subsequent steps
- Uses
set -efor proper error handling
65-67: LGTM: EKS kubeconfig setup.The EKS configuration correctly updates kubeconfig for the specified cluster and is appropriately conditional on the
aws-vpncluster input.
69-79: LGTM: Well-configured Helmfile execution.The Helmfile step is properly configured with:
- Pinned action version for security
- Explicit Helmfile and Helm versions for reproducibility
- Required plugins (helm-diff and helm-secrets) for the workflow
- Correct working directory and argument passing
closes #13
Summary by CodeRabbit
New Features
Security Improvements
Chores