Skip to content

Commit 2a994be

Browse files
committed
add comments requested in code review
Signed-off-by: Tim Ramlot <42113979+inteon@users.noreply.github.com>
1 parent 5fa2201 commit 2a994be

1 file changed

Lines changed: 11 additions & 2 deletions

File tree

pkg/datagatherer/oidc/oidc.go

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,8 @@ type DataGathererOIDC struct {
4848
cl rest.Interface
4949
}
5050

51+
var _ datagatherer.DataGatherer = &DataGathererOIDC{}
52+
5153
func (g *DataGathererOIDC) Run(ctx context.Context) error {
5254
return nil
5355
}
@@ -57,7 +59,7 @@ func (g *DataGathererOIDC) WaitForCacheSync(ctx context.Context) error {
5759
return nil
5860
}
5961

60-
// Fetch will fetch discovery data from the apiserver, or return an error
62+
// Fetch will fetch the OIDC discovery document and JWKS from the cluster API server.
6163
func (g *DataGathererOIDC) Fetch() (any, int, error) {
6264
ctx := context.Background()
6365

@@ -76,7 +78,7 @@ func (g *DataGathererOIDC) Fetch() (any, int, error) {
7678
OIDCConfigError: errToString(oidcErr),
7779
JWKS: jwksResponse,
7880
JWKSError: errToString(jwksErr),
79-
}, 1, nil
81+
}, 1 /* we have 1 result, so return 1 as count */, nil
8082
}
8183

8284
type OIDCDiscoveryData struct {
@@ -87,6 +89,7 @@ type OIDCDiscoveryData struct {
8789
}
8890

8991
func (g *DataGathererOIDC) fetchOIDCConfig(ctx context.Context) (map[string]any, error) {
92+
// Fetch the OIDC discovery document from the well-known endpoint.
9093
bytes, err := g.cl.Get().AbsPath("/.well-known/openid-configuration").Do(ctx).Raw()
9194
if err != nil {
9295
return nil, fmt.Errorf("failed to get OIDC discovery document: %v", err)
@@ -101,6 +104,12 @@ func (g *DataGathererOIDC) fetchOIDCConfig(ctx context.Context) (map[string]any,
101104
}
102105

103106
func (g *DataGathererOIDC) fetchJWKS(ctx context.Context) (map[string]any, error) {
107+
// Fetch the JWKS from the default /openid/v1/jwks endpoint.
108+
// We are not using the jwks_uri from the OIDC config because:
109+
// - on hybrid OpenShift clusters, we saw it pointed to a non-existent URL
110+
// - on fully private AWS EKS clusters, the URL is still public and might not
111+
// be reachable from within the cluster (https://github.com/aws/containers-roadmap/issues/2038)
112+
// So we are using the default path instead, which we think should work in most cases.
104113
bytes, err := g.cl.Get().AbsPath("/openid/v1/jwks").Do(ctx).Raw()
105114
if err != nil {
106115
return nil, fmt.Errorf("failed to get JWKS from jwks_uri: %v", err)

0 commit comments

Comments
 (0)