-
Notifications
You must be signed in to change notification settings - Fork 190
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 helm unittest #334
Feat helm unittest #334
Conversation
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.
This is fantastic work. You beat me to it!
@@ -0,0 +1,48 @@ | |||
--- | |||
suite: test secret-aws for aws |
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 mind also adding a github action for this?
# .github/workflows/tests.yaml
name: tests
on: pull_request
jobs:
unittest:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: d3adb5/helm-unittest-action@v2
with:
# https://github.com/helm/helm/releases
helm-version: v3.13.2
github-token: ${{ secrets.GITHUB_TOKEN }}
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.
If this job is added, then it needs to be removed from ct lint
Lines 7 to 8 in f400585
additional-commands: | |
- helm unittest {{ .Path }} |
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 awesome! Some minor remarks 😃
@@ -10,7 +10,7 @@ data: | |||
key.pem: {{ required "githubApp.key is required if githubApp configuration is specified." .Values.githubApp.key | b64enc }} | |||
github_secret: {{ required "githubApp.secret is required if githubApp configuration is specified." .Values.githubApp.secret | b64enc }} | |||
{{- end}} | |||
{{- if .Values.github.user }} |
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.
While this makes sense, I would change this on a different PR. Also since there are other places where this can be improved [(example)](
helm-charts/charts/atlantis/templates/statefulset.yaml
Lines 296 to 309 in 94ff3d4
{{- if .Values.github.user }} | |
- name: ATLANTIS_GH_USER | |
value: {{ required "github.user is required if github configuration is specified." .Values.github.user }} | |
- name: ATLANTIS_GH_TOKEN | |
valueFrom: | |
secretKeyRef: | |
name: {{ template "atlantis.vcsSecretName" . }} | |
key: github_token | |
- name: ATLANTIS_GH_WEBHOOK_SECRET | |
valueFrom: | |
secretKeyRef: | |
name: {{ template "atlantis.vcsSecretName" . }} | |
key: github_secret | |
{{- end }} |
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.
Removed it will be taken in another pr
charts/atlantis/values.schema.json
Outdated
"orgWhitelist":{ | ||
"type":"string", | ||
"default":"<replace-me>", | ||
"description":"Deprecated (see orgAllowlist) List of repositories from which Atlantis will accept webhooks. Accepts wildcard characters (`*`). Multiple values may be comma-separated.", | ||
"examples":[ | ||
"github.com/myorg/*" | ||
] | ||
}, |
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.
This is another example of a change that would be better in a different PR 😃
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 be fixed in another pr
what
why
tests
references