argo-cd icon indicating copy to clipboard operation
argo-cd copied to clipboard

fix: Suppressed ssh scheme url warn log

Open kenchan0130 opened this issue 1 year ago • 5 comments

Fixes #9440

The resolution of the above issue, i.e., the degrade, has been resolved; the root resolution of https://github.com/argoproj/argo-cd/issues/9440#issuecomment-1165099222 is outside the scope of this PR.

Note on DCO:

If the DCO action in the integration test fails, one or more of your commits are not signed off. Please click on the Details link next to the DCO action for instructions on how to resolve this.

Checklist:

  • [x] Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes.
  • [x] The title of the PR states what changed and the related issues number (used for the release note).
  • [x] I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue.
  • [ ] I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them.
  • [ ] Does this PR require documentation updates?
  • [ ] I've updated documentation as required by this PR.
  • [ ] Optional. My organization is added to USERS.md.
  • [x] I have signed off all my commits as required by DCO
  • [x] I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
  • [ ] My build is green (troubleshooting builds).

kenchan0130 avatar Jun 30 '22 15:06 kenchan0130

Codecov Report

Merging #9836 (ed7195c) into master (a588e63) will increase coverage by 0.07%. The diff coverage is 72.22%.

@@            Coverage Diff             @@
##           master    #9836      +/-   ##
==========================================
+ Coverage   45.81%   45.88%   +0.07%     
==========================================
  Files         227      227              
  Lines       26882    27381     +499     
==========================================
+ Hits        12315    12564     +249     
- Misses      12891    13111     +220     
- Partials     1676     1706      +30     
Impacted Files Coverage Δ
pkg/apis/application/v1alpha1/repository_types.go 68.60% <72.22%> (-0.54%) :arrow_down:
util/dex/dex.go 57.14% <0.00%> (-30.74%) :arrow_down:
util/app/path/path.go 66.66% <0.00%> (-17.95%) :arrow_down:
util/grpc/useragent.go 45.83% <0.00%> (-10.42%) :arrow_down:
server/application/websocket.go 7.54% <0.00%> (-5.36%) :arrow_down:
applicationset/generators/cluster.go 74.07% <0.00%> (-4.06%) :arrow_down:
util/env/env.go 86.95% <0.00%> (-3.96%) :arrow_down:
util/dex/config.go 85.56% <0.00%> (-3.45%) :arrow_down:
util/tls/tls.go 74.66% <0.00%> (-3.33%) :arrow_down:
server/server.go 51.49% <0.00%> (-3.17%) :arrow_down:
... and 61 more

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 a588e63...ed7195c. Read the comment docs.

codecov[bot] avatar Jun 30 '22 16:06 codecov[bot]

The mage / publish (pull_request) fail does not seem to be related to this change.

kenchan0130 avatar Jul 04 '22 01:07 kenchan0130

Could you review this PR?

cc: @crenshaw-dev (because it was commented on in the related issue)

kenchan0130 avatar Jul 26 '22 03:07 kenchan0130

@kenchan0130 just trying to make sure I understand. The tested "invalid" path is in fact a valid repo.Repo value, correct? So won't this just mean that constructing the CA path will silently fail when it is potentially needed? In other words, it might push people to use insecure when validation fails.

crenshaw-dev avatar Jul 27 '22 18:07 crenshaw-dev

@crenshaw-dev Thank you for your comment. I modified the test to fix the problem.

kenchan0130 avatar Jul 28 '22 15:07 kenchan0130

Cherry-picked onto release-2.4 for 2.4.10.

crenshaw-dev avatar Aug 15 '22 16:08 crenshaw-dev