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

fix: do not unset passCredentials when it's not specified (#9102) #9104

Merged
merged 6 commits into from
Apr 18, 2022

Conversation

crenshaw-dev
Copy link
Collaborator

Fixes #9102

The main issue was that passCredentials was always unset.

The other issue was that some fields were being unset even if we didn't first check if they needed to be unset. So we were making an unnecessary API call.

)

Signed-off-by: Michael Crenshaw <michael@crenshaw.dev>
@codecov
Copy link

codecov bot commented Apr 14, 2022

Codecov Report

Merging #9104 (5db78af) into master (67e45d8) will increase coverage by 0.15%.
The diff coverage is 75.00%.

@@            Coverage Diff             @@
##           master    #9104      +/-   ##
==========================================
+ Coverage   45.51%   45.66%   +0.15%     
==========================================
  Files         217      217              
  Lines       25663    25658       -5     
==========================================
+ Hits        11680    11717      +37     
+ Misses      12355    12316      -39     
+ Partials     1628     1625       -3     
Impacted Files Coverage Δ
cmd/argocd/commands/app.go 12.40% <75.00%> (+3.23%) ⬆️
applicationset/services/scm_provider/github.go 63.52% <0.00%> (-17.65%) ⬇️
util/settings/settings.go 48.10% <0.00%> (ø)
applicationset/services/scm_provider/utils.go 88.50% <0.00%> (+4.59%) ⬆️

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 67e45d8...5db78af. Read the comment docs.

Signed-off-by: Michael Crenshaw <michael@crenshaw.dev>
Signed-off-by: Michael Crenshaw <michael@crenshaw.dev>
Signed-off-by: Michael Crenshaw <michael@crenshaw.dev>
@pasha-codefresh
Copy link
Member

i will review it in few hours @crenshaw-dev

Copy link
Member

@pasha-codefresh pasha-codefresh 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

@pasha-codefresh pasha-codefresh merged commit 2738358 into argoproj:master Apr 18, 2022
@crenshaw-dev crenshaw-dev deleted the fix-app-unset branch April 18, 2022 22:02
wojtekidd pushed a commit to wojtekidd/argo-cd that referenced this pull request Apr 25, 2022
) (argoproj#9104)

* fix: do not unset passCredentials when it's not specified (argoproj#9102)

Signed-off-by: Michael Crenshaw <michael@crenshaw.dev>

* chore: codegen

Signed-off-by: Michael Crenshaw <michael@crenshaw.dev>

* chore: more tests, no-update detection for kustomize

Signed-off-by: Michael Crenshaw <michael@crenshaw.dev>

* chore: fix test

Signed-off-by: Michael Crenshaw <michael@crenshaw.dev>
Signed-off-by: wojtekidd <wojtek.cichon@protonmail.com>
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.

argocd app unset always unsets helm.passCredentials
2 participants