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

config: Allow alternative kubernetes.default.svc DNS names. #1288

Closed
wants to merge 1 commit into from

Conversation

wlynch
Copy link
Member

@wlynch wlynch commented Jul 21, 2023

Summary

Sometimes scaffolding will fail due to minor differences in the local OIDC issuers, even though these are functionally the same.

2023-07-21T21:59:29.941Z        FATAL   app/serve.go:209        error loading --config-path=/etc/fulcio-config/config.json: provider https://kubernetes.default.svc: oidc: issuer did not match the issuer returned by provider, expected "https://kubernetes.default.svc" got "https://kubernetes.default.svc.cluster.local"

This adds support for https://kubernetes.default.svc.cluster.local, and will load the cluster CA to the transport if either are present.

Also removes the originalTransport global var, since it isn't being used in a meaningful way.

cc @vaikas

Release Note

Adds support for alternative local-kubernetes OIDC issuer DNS names (e.g. kubernetes.default.svc vs kubernetes.default.svc.cluster.local)

Documentation

Sometimes scaffolding will fail due to minor differences in the local
OIDC issuers, even though these are functionally the same.

This adds support for https://kubernetes.default.svc.cluster.local, and
will load the cluster CA to the transport if either are present.

Signed-off-by: Billy Lynch <billy@chainguard.dev>
@codecov
Copy link

codecov bot commented Jul 21, 2023

Codecov Report

Merging #1288 (b01918b) into main (34b7f40) will increase coverage by 0.09%.
The diff coverage is 25.00%.

@@            Coverage Diff             @@
##             main    #1288      +/-   ##
==========================================
+ Coverage   58.23%   58.33%   +0.09%     
==========================================
  Files          50       50              
  Lines        3053     3053              
==========================================
+ Hits         1778     1781       +3     
+ Misses       1116     1114       -2     
+ Partials      159      158       -1     
Impacted Files Coverage Δ
pkg/config/config.go 74.23% <25.00%> (ø)

... and 1 file with indirect coverage changes

t := originalTransport.(*http.Transport).Clone()
t.TLSClientConfig.RootCAs = rootCAs
http.DefaultTransport = t
} else {
Copy link
Contributor

Choose a reason for hiding this comment

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

@wlynch Can you confirm that this else condition is no longer needed? I don't see a place where we overwrote it previously, but I don't have context into why it was there initially.

@haydentherapper
Copy link
Contributor

haydentherapper commented Aug 1, 2023

Bump, is this still needed?

@wlynch
Copy link
Member Author

wlynch commented Aug 4, 2023

I actually don't know 😅

This was partially in response to some of the issues we've been seeing in scaffolding. I'll close this for now, we can always reopen 🤷

@wlynch wlynch closed this Aug 4, 2023
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