Skip to content

Conversation

@spoukke
Copy link
Contributor

@spoukke spoukke commented Jun 2, 2021

The default behavior of image updater is to use argocd credentials. If it is setup with ssh, this volume mounts and volume are needed in order for image updater to be able to commit to the repository

@CLAassistant
Copy link

CLAassistant commented Jun 2, 2021

CLA assistant check
All committers have signed the CLA.

@codecov
Copy link

codecov bot commented Jun 7, 2021

Codecov Report

Merging #212 (c509013) into master (add387e) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #212   +/-   ##
=======================================
  Coverage   66.77%   66.77%           
=======================================
  Files          20       20           
  Lines        1574     1574           
=======================================
  Hits         1051     1051           
  Misses        424      424           
  Partials       99       99           

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 add387e...c509013. Read the comment docs.

Copy link
Contributor

@jannfis jannfis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this PR @spoukke and sorry for the late review.

I feel this mount should be marked as optional, because there will be scenarios where this CM does not exist (e.g. user installs the image updater outside argocd namespace). Can you please change it so it's an optional mount? Thanks.

@spoukke
Copy link
Contributor Author

spoukke commented Jun 7, 2021

Thanks for this PR @spoukke and sorry for the late review.

I feel this mount should be marked as optional, because there will be scenarios where this CM does not exist (e.g. user installs the image updater outside argocd namespace). Can you please change it so it's an optional mount? Thanks.

Hi @jannfis ! No worries. The change is done, let me know if there is anything else

Copy link
Contributor

@jannfis jannfis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you @spoukke !

@jannfis jannfis merged commit 7708118 into argoproj-labs:master Jun 11, 2021
sribiere-jellysmack pushed a commit to sribiere-jellysmack/argocd-image-updater that referenced this pull request Aug 13, 2024
* feat(manifests): add ssh volume mounts and volume

* feat(ssh-volume): add optionnal mount
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