fix(chart): dedup imagePullSecrets in trillian and rekor

ndegory commented 5 months ago

Description of the change

imagePullSecrets is duplicated in the Trillian chart, which fails the rendering when trying to set an image pull secret.

Existing or Associated Issue(s)

Closes #738

Additional Information

➜ ct lint --lint-conf ./ct.yaml
Linting charts...
------------------------------------------------------------------------------------------------------------------------
 Charts to be processed:
------------------------------------------------------------------------------------------------------------------------
 rekor => (version: "1.4.1", path: "charts/rekor")
 trillian => (version: "0.2.23", path: "charts/trillian")
------------------------------------------------------------------------------------------------------------------------
"sigstore" already exists with the same configuration, skipping                                                                                                                                                                       
Linting chart "rekor => (version: \"1.4.1\", path: \"charts/rekor\")"
Checking chart "rekor => (version: \"1.4.1\", path: \"charts/rekor\")" for a version bump...
Old chart version: 1.4.0
New chart version: 1.4.1
Chart version ok.
Validating /Users/nicolas.degory/Code/src/github.com/sigstore/helm-charts/charts/rekor/Chart.yaml...
Validation success! 👍

Linting chart with values file "charts/rekor/ci/ci-values.yaml"...

==> Linting charts/rekor
[INFO] Chart.yaml: icon is recommended

1 chart(s) linted, 0 chart(s) failed
Linting chart "trillian => (version: \"0.2.23\", path: \"charts/trillian\")"
Checking chart "trillian => (version: \"0.2.23\", path: \"charts/trillian\")" for a version bump...
Old chart version: 0.2.22
New chart version: 0.2.23
Chart version ok.
Validating /Users/nicolas.degory/Code/src/github.com/sigstore/helm-charts/charts/trillian/Chart.yaml...
Validation success! 👍
==> Linting charts/trillian
[INFO] Chart.yaml: icon is recommended

1 chart(s) linted, 0 chart(s) failed

------------------------------------------------------------------------------------------------------------------------
 ✔︎ rekor => (version: "1.4.1", path: "charts/rekor")
 ✔︎ trillian => (version: "0.2.23", path: "charts/trillian")
------------------------------------------------------------------------------------------------------------------------
All charts linted successfully

I haven't updated the dependencies in scaffold and rekor, I suppose this PR has to be merged first and the trillian chart published to be able to update the Chart.lock.

Checklist

[x] Chart version bumped in Chart.yaml according to semver. Where applicable, update and bump the versions in any associated umbrella chart
[ ] Variables are documented in the values.yaml and added to the README.md. The helm-docs utility can be used to generate the necessary content. Use helm-docs --dry-run to preview the content.
[ ] JSON Schema generated.
[x] List tests pass for Chart using the Chart Testing tool and the ct lint command.

hectorj2f commented 5 months ago

@ndegory Where do you see it duplicated in https://github.com/sigstore/helm-charts/blob/f02b411e081467206d680e601779f1c0c15ff024/charts/trillian/templates/mysql/deployment.yaml ? With your change, we are removing it from the deployment.

ndegory commented 5 months ago

@hectorj2f this file is after I removed one of the two imagePullSecrets from the template, and you can see one is left on line 46

ndegory commented 5 months ago

@hectorj2f , do we agree there's a duplication in current version of the trillian chart?

ndegory commented 4 months ago

@hectorj2f , this issue is now also in the latest rekor chart, you can see the double definition at these two lines: https://github.com/sigstore/helm-charts/blob/rekor-1.4.0/charts/rekor/templates/mysql/deployment.yaml#L46 https://github.com/sigstore/helm-charts/blob/rekor-1.4.0/charts/rekor/templates/mysql/deployment.yaml#L99

cpanato commented 4 months ago

@ndegory please sign the DCO and split the changes in two PRs as they are for two different charts, thanks for the contribution!

ndegory commented 4 months ago

looks like @cmurphy applied the same fix in #752 and got approved, so I'll close this one.

cmurphy commented 4 months ago

@ndegory sorry about that! Should have checked first.

haydentherapper commented 4 months ago

Thank you @ndegory for raising this!

sigstore / helm-charts