Skip to content
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: allow write-back to actual kustomization files #200

Merged

Conversation

iamnoah
Copy link
Contributor

@iamnoah iamnoah commented Apr 30, 2021

fixes #199

I don't love the new type and annotations, this feels like a sub-option of the git method, but I don't have a great idea for better options.

@codecov
Copy link

codecov bot commented Apr 30, 2021

Codecov Report

Merging #200 (89f3012) into master (b47f7e9) will increase coverage by 0.06%.
The diff coverage is 58.16%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #200      +/-   ##
==========================================
+ Coverage   66.71%   66.77%   +0.06%     
==========================================
  Files          20       20              
  Lines        1517     1574      +57     
==========================================
+ Hits         1012     1051      +39     
- Misses        412      424      +12     
- Partials       93       99       +6     
Impacted Files Coverage Δ
pkg/argocd/git.go 60.95% <51.94%> (+1.15%) ⬆️
pkg/argocd/update.go 70.56% <80.95%> (+0.83%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b47f7e9...89f3012. Read the comment docs.

@jannfis
Copy link
Contributor

jannfis commented Apr 30, 2021

Hey, how about this. We do have write-back-method annotation for specifying the method of writing back (e.g. argocd and git currently), we could have an additional write-back-target annotation to specify the target of the write-back.

That new annotation could have values override[:<app-source.yaml>] (default, uses the .app-source.yaml) or kustomize[:<kustomization.yaml>], each with an optional parameter (after a colon) to specify which file to actually override - e.g. some people don't use kustomization.yaml but Kustomization.yaml or even plain Kustomization. And maybe it can also be used to specify some path within the repository to use.

Oh, and never mind about implementing the override option with this PR, we can introduce it along with parameters later on. We should just make sure that the default behavior stays the same after this change (e.g. writes an app source override if write-back-target is not given).

@jannfis
Copy link
Contributor

jannfis commented Apr 30, 2021

Regarding the spell checking of the docs, please add the "unknown" words it complains about (if they are not typos after all) to https://github.com/argoproj-labs/argocd-image-updater/blob/master/.github/actions/spelling/allow.txt

@iamnoah iamnoah force-pushed the kustomization-write-back branch 2 times, most recently from 2928685 to 352646b Compare May 1, 2021 01:34
@jannfis
Copy link
Contributor

jannfis commented May 6, 2021

Hey @iamnoah, can you please take a look at the complaints of the linter (as described here)

@jannfis
Copy link
Contributor

jannfis commented May 11, 2021

LGTM, thanks @iamnoah

@jannfis jannfis merged commit add387e into argoproj-labs:master May 11, 2021
@iamnoah iamnoah deleted the kustomization-write-back branch May 17, 2021 20:29
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.

Write Back to kustomization.yml
2 participants