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.

Sign up for GitHub

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

Jump to bottom

Client key and cert should not be required. #3407

Closed
wants to merge 2 commits into from
Closed

Client key and cert should not be required. #3407

wants to merge 2 commits into from

Conversation

scr-oath
Copy link
Contributor

Fixing schema change from #3302

Signed-off-by: Sheridan C Rawlins scr@yahooinc.com

...

...

...

@scr-oath
Client key and cert should not be required.
4b1784e 
Fixing schema change from goreleaser#3302

Signed-off-by: Sheridan C Rawlins <scr@yahooinc.com>
@pull-request-size pull-request-size bot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Sep 27, 2022
@scr-oath
Also add the omitempty field for the json of client cert and key.
46416c2 
Signed-off-by: Sheridan C Rawlins <scr@yahooinc.com>
@scr-oath scr-oath force-pushed the client-key-and-cert-should-not-be-required branch from 3f5ead1 to 46416c2 Compare September 27, 2022 04:59
@codecov
Copy link

codecov bot commented Sep 27, 2022 •
edited

Codecov Report

Merging #3407 (3f5ead1) into main (a8b3806) will not change coverage.
The diff coverage is n/a.

❗ Current head 3f5ead1 differs from pull request most recent head 46416c2. Consider uploading reports for the commit 46416c2 to get more accurate results

@@           Coverage Diff           @@
##             main    #3407   +/-   ##
=======================================
  Coverage   83.86%   83.86%           
=======================================
  Files         115      115           
  Lines        9218     9218           
=======================================
  Hits         7731     7731           
  Misses       1210     1210           
  Partials      277      277
Impacted Files Coverage Δ
pkg/config/config.go 95.21% <ø> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@caarlos0 caarlos0 mentioned this pull request Sep 27, 2022
Merged
@caarlos0 caarlos0 added the bug Something isn't working label Sep 27, 2022
@caarlos0
Copy link
Member

format fixed on #3409

caarlos0 added a commit that referenced this pull request Sep 27, 2022
@caarlos0 @scr-oath
fix: client key and cert should not be required (#3409)
f3849e2 
closes #3407

Signed-off-by: Sheridan C Rawlins <scr@yahooinc.com>
Signed-off-by: Carlos A Becker <caarlos0@users.noreply.github.com>
Co-authored-by: Sheridan C Rawlins <scr@yahooinc.com>
@caarlos0
Copy link
Member

Thanks!

@scr-oath
Copy link
Contributor Author

FYI if curious, this cert stuff does work for mTLS protected sites; I was just surprised that I had made it required before (which could have broken others that use basic auth, or at least confused them in smart IDEs that look at the schema)

@caarlos0
Copy link
Member

FYI if curious, this cert stuff does work for mTLS protected sites; I was just surprised that I had made it required before (which could have broken others that use basic auth, or at least confused them in smart IDEs that look at the schema)

it was just the yamlschema wrong, the code would work fine without the certs for non mtls stuff

@github-actions
Copy link
Contributor

github-actions bot commented Dec 5, 2022

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
bug Something isn't working size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@scr-oath @caarlos0