Skip to content

Update helm charts beyond values.yaml#6097

Merged
jeffwidman merged 1 commit into
mainfrom
update-helm-charts-beyond-values.yml
Nov 11, 2022
Merged

Update helm charts beyond values.yaml#6097
jeffwidman merged 1 commit into
mainfrom
update-helm-charts-beyond-values.yml

Conversation

@jeffwidman
Copy link
Copy Markdown
Member

The docker updater for helm charts was hardcoded to "values.yaml".

But we should support the full range of filenames supported in the file fetcher... for example, in the wild we saw this failing to update "arc-values.yaml" because of this hardcoding.

@jeffwidman
Copy link
Copy Markdown
Member Author

Need tests for the updater to ensure we're processing this full suite of files that we do in the fetcher:

matching_filenames = [
"other-values.yml",
"other-values.yaml",
"other_values.yml",
"other_values.yaml",
"values.yml",
"values.yaml",
"values-other.yml",
"values-other.yaml",
"values_other.yml",
"values_other.yaml",
"values2.yml",
"values2.yaml"
]

The docker updater for helm charts was hardcoded to `"values.yaml"`.

But we should support the full range of filenames supported in the file
fetcher... for example, in the wild we saw this failing to update
`"arc-values.yaml"` because of this hardcoding.
@jeffwidman jeffwidman force-pushed the update-helm-charts-beyond-values.yml branch from 39dc4b0 to 1156dc1 Compare November 10, 2022 22:22
@jeffwidman jeffwidman marked this pull request as ready for review November 10, 2022 23:07
@jeffwidman jeffwidman requested a review from a team as a code owner November 10, 2022 23:07
@jeffwidman
Copy link
Copy Markdown
Member Author

I broke the unit tests out separately into #6105 so that it's not blocking merging this code... I'm still learning the Rspec DSL, but since this code is behind a feature flag I'd like to get it live sooner so that if there are other bugs/issues I can see them sooner even while I work through the unit tests. Also so that I can continue to test this code w/o being blocked because other maintainers are offline for Veteran's day.

@jeffwidman
Copy link
Copy Markdown
Member Author

I manually confirmed this is working against the previously failing repo:

[dependabot-core-dev] ~/dependabot-core $ LOCAL_GITHUB_ACCESS_TOKEN=supersecret bin/dry-run.rb --dir "/manifest_staging/charts/csi-secrets-store-provider-azure" docker jeffwidman/secrets-store-csi-driver-provider-azure --updater-options=kubernetes_updates

...

 => requirements update strategy:
 => updating oss/kubernetes-csi/secrets-store/driver from v1.2.3 to v1.2.4.1

    ± arc-values.yaml
    ~~~
    115c115
    <       tag: v1.2.3
    ---
    >       tag: v1.2.4.1
    128c128
    <         tag: v1.2.3
    ---
    >         tag: v1.2.4.1
    140c140
    <       tag: v1.2.3
    ---
    >       tag: v1.2.4.1
    ~~~

    ± values.yaml
    ~~~
    117c117
    <       tag: v1.2.3
    ---
    >       tag: v1.2.4.1
    130c130
    <         tag: v1.2.3
    ---
    >         tag: v1.2.4.1
    141c141
    <       tag: v1.2.3
    ---
    >       tag: v1.2.4.1
    ~~~
🌍 Total requests made: '0'

@jeffwidman jeffwidman merged commit 414ac67 into main Nov 11, 2022
@jeffwidman jeffwidman deleted the update-helm-charts-beyond-values.yml branch November 11, 2022 01:55
@jeffwidman
Copy link
Copy Markdown
Member Author

This has been deployed. I'll finish up the unit tests shortly in the other PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants