From 5fa220143761c4040ad7f61a843895b881f21131 Mon Sep 17 00:00:00 2001 From: Tim Ramlot <42113979+inteon@users.noreply.github.com> Date: Thu, 22 Jan 2026 17:40:54 +0100 Subject: [PATCH 1/2] add OIDC datagatherer Signed-off-by: Tim Ramlot <42113979+inteon@users.noreply.github.com> --- .../disco-agent/templates/configmap.yaml | 2 + deploy/charts/disco-agent/templates/rbac.yaml | 16 ++- .../__snapshot__/configmap_test.yaml.snap | 8 ++ examples/one-shot-oidc.yaml | 16 +++ pkg/agent/config.go | 3 + pkg/datagatherer/oidc/oidc.go | 115 +++++++++++++++++ pkg/datagatherer/oidc/oidc_test.go | 119 ++++++++++++++++++ 7 files changed, 278 insertions(+), 1 deletion(-) create mode 100644 examples/one-shot-oidc.yaml create mode 100644 pkg/datagatherer/oidc/oidc.go create mode 100644 pkg/datagatherer/oidc/oidc_test.go diff --git a/deploy/charts/disco-agent/templates/configmap.yaml b/deploy/charts/disco-agent/templates/configmap.yaml index 231a26cd..4766e762 100644 --- a/deploy/charts/disco-agent/templates/configmap.yaml +++ b/deploy/charts/disco-agent/templates/configmap.yaml @@ -19,6 +19,8 @@ data: {{- . | toYaml | nindent 6 }} {{- end }} data-gatherers: + - kind: oidc + name: ark/oidc - kind: k8s-discovery name: ark/discovery - kind: k8s-dynamic diff --git a/deploy/charts/disco-agent/templates/rbac.yaml b/deploy/charts/disco-agent/templates/rbac.yaml index f3fae414..cc8ca8aa 100644 --- a/deploy/charts/disco-agent/templates/rbac.yaml +++ b/deploy/charts/disco-agent/templates/rbac.yaml @@ -95,4 +95,18 @@ subjects: - kind: ServiceAccount name: {{ include "disco-agent.serviceAccountName" . }} namespace: {{ .Release.Namespace }} - +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +metadata: + name: {{ include "disco-agent.fullname" . }}-oidc-discovery + labels: + {{- include "disco-agent.labels" . | nindent 4 }} +roleRef: + kind: ClusterRole + name: system:service-account-issuer-discovery + apiGroup: rbac.authorization.k8s.io +subjects: + - kind: ServiceAccount + name: {{ include "disco-agent.serviceAccountName" . }} + namespace: {{ .Release.Namespace }} diff --git a/deploy/charts/disco-agent/tests/__snapshot__/configmap_test.yaml.snap b/deploy/charts/disco-agent/tests/__snapshot__/configmap_test.yaml.snap index 2c70df00..89a88ed3 100644 --- a/deploy/charts/disco-agent/tests/__snapshot__/configmap_test.yaml.snap +++ b/deploy/charts/disco-agent/tests/__snapshot__/configmap_test.yaml.snap @@ -7,6 +7,8 @@ custom-cluster-description: cluster_description: "A cloud hosted Kubernetes cluster hosting production workloads.\n\nteam: team-1\nemail: team-1@example.com\npurpose: Production workloads\n" period: "12h0m0s" data-gatherers: + - kind: oidc + name: ark/oidc - kind: k8s-discovery name: ark/discovery - kind: k8s-dynamic @@ -114,6 +116,8 @@ custom-cluster-name: cluster_description: "" period: "12h0m0s" data-gatherers: + - kind: oidc + name: ark/oidc - kind: k8s-discovery name: ark/discovery - kind: k8s-dynamic @@ -221,6 +225,8 @@ custom-period: cluster_description: "" period: "1m" data-gatherers: + - kind: oidc + name: ark/oidc - kind: k8s-discovery name: ark/discovery - kind: k8s-dynamic @@ -328,6 +334,8 @@ defaults: cluster_description: "" period: "12h0m0s" data-gatherers: + - kind: oidc + name: ark/oidc - kind: k8s-discovery name: ark/discovery - kind: k8s-dynamic diff --git a/examples/one-shot-oidc.yaml b/examples/one-shot-oidc.yaml new file mode 100644 index 00000000..d5ce8e99 --- /dev/null +++ b/examples/one-shot-oidc.yaml @@ -0,0 +1,16 @@ +# one-shot-oidc.yaml +# +# An example configuration file which can be used for local testing. +# For example: +# +# go run . agent \ +# --agent-config-file examples/one-shot-oidc.yaml \ +# --one-shot \ +# --output-path output.json +# +organization_id: "my-organization" +cluster_id: "my_cluster" +period: 1m +data-gatherers: +- kind: "oidc" + name: "ark/oidc" diff --git a/pkg/agent/config.go b/pkg/agent/config.go index ed72afe4..b63e09cf 100644 --- a/pkg/agent/config.go +++ b/pkg/agent/config.go @@ -22,6 +22,7 @@ import ( "github.com/jetstack/preflight/pkg/datagatherer/k8sdiscovery" "github.com/jetstack/preflight/pkg/datagatherer/k8sdynamic" "github.com/jetstack/preflight/pkg/datagatherer/local" + "github.com/jetstack/preflight/pkg/datagatherer/oidc" "github.com/jetstack/preflight/pkg/kubeconfig" "github.com/jetstack/preflight/pkg/logs" "github.com/jetstack/preflight/pkg/version" @@ -901,6 +902,8 @@ func (dg *DataGatherer) UnmarshalYAML(unmarshal func(any) error) error { cfg = &k8sdynamic.ConfigDynamic{} case "k8s-discovery": cfg = &k8sdiscovery.ConfigDiscovery{} + case "oidc": + cfg = &oidc.OIDCDiscovery{} case "local": cfg = &local.Config{} // dummy dataGatherer is just used for testing diff --git a/pkg/datagatherer/oidc/oidc.go b/pkg/datagatherer/oidc/oidc.go new file mode 100644 index 00000000..d68fbfc9 --- /dev/null +++ b/pkg/datagatherer/oidc/oidc.go @@ -0,0 +1,115 @@ +package oidc + +import ( + "context" + "encoding/json" + "fmt" + + "k8s.io/client-go/rest" + + "github.com/jetstack/preflight/pkg/datagatherer" + "github.com/jetstack/preflight/pkg/kubeconfig" +) + +// OIDCDiscovery contains the configuration for the k8s-discovery data-gatherer +type OIDCDiscovery struct { + // KubeConfigPath is the path to the kubeconfig file. If empty, will assume it runs in-cluster. + KubeConfigPath string `yaml:"kubeconfig"` +} + +// UnmarshalYAML unmarshals the Config resolving GroupVersionResource. +func (c *OIDCDiscovery) UnmarshalYAML(unmarshal func(any) error) error { + aux := struct { + KubeConfigPath string `yaml:"kubeconfig"` + }{} + err := unmarshal(&aux) + if err != nil { + return err + } + + c.KubeConfigPath = aux.KubeConfigPath + + return nil +} + +func (c *OIDCDiscovery) NewDataGatherer(ctx context.Context) (datagatherer.DataGatherer, error) { + cl, err := kubeconfig.NewDiscoveryClient(c.KubeConfigPath) + if err != nil { + return nil, err + } + + return &DataGathererOIDC{ + cl: cl.RESTClient(), + }, nil +} + +// DataGathererOIDC stores the config for a k8s-discovery datagatherer +type DataGathererOIDC struct { + cl rest.Interface +} + +func (g *DataGathererOIDC) Run(ctx context.Context) error { + return nil +} + +func (g *DataGathererOIDC) WaitForCacheSync(ctx context.Context) error { + // no async functionality, see Fetch + return nil +} + +// Fetch will fetch discovery data from the apiserver, or return an error +func (g *DataGathererOIDC) Fetch() (any, int, error) { + ctx := context.Background() + + oidcResponse, oidcErr := g.fetchOIDCConfig(ctx) + jwksResponse, jwksErr := g.fetchJWKS(ctx) + + errToString := func(err error) string { + if err != nil { + return err.Error() + } + return "" + } + + return OIDCDiscoveryData{ + OIDCConfig: oidcResponse, + OIDCConfigError: errToString(oidcErr), + JWKS: jwksResponse, + JWKSError: errToString(jwksErr), + }, 1, nil +} + +type OIDCDiscoveryData struct { + OIDCConfig map[string]any `json:"openid_configuration,omitempty"` + OIDCConfigError string `json:"openid_configuration_error,omitempty"` + JWKS map[string]any `json:"jwks,omitempty"` + JWKSError string `json:"jwks_error,omitempty"` +} + +func (g *DataGathererOIDC) fetchOIDCConfig(ctx context.Context) (map[string]any, error) { + bytes, err := g.cl.Get().AbsPath("/.well-known/openid-configuration").Do(ctx).Raw() + if err != nil { + return nil, fmt.Errorf("failed to get OIDC discovery document: %v", err) + } + + var oidcResponse map[string]any + if err := json.Unmarshal(bytes, &oidcResponse); err != nil { + return nil, fmt.Errorf("failed to unmarshal OIDC discovery document: %v", err) + } + + return oidcResponse, nil +} + +func (g *DataGathererOIDC) fetchJWKS(ctx context.Context) (map[string]any, error) { + bytes, err := g.cl.Get().AbsPath("/openid/v1/jwks").Do(ctx).Raw() + if err != nil { + return nil, fmt.Errorf("failed to get JWKS from jwks_uri: %v", err) + } + + var jwksResponse map[string]any + if err := json.Unmarshal(bytes, &jwksResponse); err != nil { + return nil, fmt.Errorf("failed to unmarshal JWKS response: %v", err) + } + + return jwksResponse, nil +} diff --git a/pkg/datagatherer/oidc/oidc_test.go b/pkg/datagatherer/oidc/oidc_test.go new file mode 100644 index 00000000..a603ee8e --- /dev/null +++ b/pkg/datagatherer/oidc/oidc_test.go @@ -0,0 +1,119 @@ +package oidc + +import ( + "net/http" + "net/http/httptest" + "net/url" + "testing" + + "k8s.io/client-go/discovery" + "k8s.io/client-go/rest" +) + +func makeRESTClient(t *testing.T, ts *httptest.Server) rest.Interface { + t.Helper() + u, err := url.Parse(ts.URL) + if err != nil { + t.Fatalf("parse server url: %v", err) + } + + cfg := &rest.Config{ + Host: u.Host, + } + + discoveryClient, err := discovery.NewDiscoveryClientForConfigAndClient(cfg, ts.Client()) + if err != nil { + t.Fatalf("new discovery client: %v", err) + } + + return discoveryClient.RESTClient() +} + +func TestFetch_Success(t *testing.T) { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/.well-known/openid-configuration": + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"issuer":"https://example"}`)) + case "/openid/v1/jwks": + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"keys":[]}`)) + default: + http.NotFound(w, r) + } + })) + defer ts.Close() + + rc := makeRESTClient(t, ts) + g := &DataGathererOIDC{cl: rc} + + anyRes, count, err := g.Fetch() + if err != nil { + t.Fatalf("Fetch returned error: %v", err) + } + if count != 1 { + t.Fatalf("expected count 1, got %d", count) + } + + res, ok := anyRes.(OIDCDiscoveryData) + if !ok { + t.Fatalf("unexpected result type: %T", anyRes) + } + + if res.OIDCConfig == nil { + t.Fatalf("expected OIDCConfig, got nil") + } + if iss, _ := res.OIDCConfig["issuer"].(string); iss != "https://example" { + t.Fatalf("unexpected issuer: %v", res.OIDCConfig["issuer"]) + } + + if res.JWKS == nil { + t.Fatalf("expected JWKS, got nil") + } + if _, ok := res.JWKS["keys"].([]any); !ok { + t.Fatalf("expected keys to be a slice, got %#v", res.JWKS["keys"]) + } +} + +func TestFetch_Errors(t *testing.T) { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/.well-known/openid-configuration": + // return server error + http.Error(w, "boom", http.StatusInternalServerError) + case "/openid/v1/jwks": + // return invalid JSON + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`}{`)) + default: + http.NotFound(w, r) + } + })) + defer ts.Close() + + rc := makeRESTClient(t, ts) + g := &DataGathererOIDC{cl: rc} + + anyRes, _, err := g.Fetch() + if err != nil { + t.Fatalf("Fetch returned error: %v", err) + } + + res, ok := anyRes.(OIDCDiscoveryData) + if !ok { + t.Fatalf("unexpected result type: %T", anyRes) + } + + if res.OIDCConfig != nil { + t.Fatalf("expected nil OIDCConfig on error, got %#v", res.OIDCConfig) + } + if res.OIDCConfigError != "failed to get OIDC discovery document: an error on the server (\"boom\") has prevented the request from succeeding" { + t.Fatalf("unexpected OIDCConfigError: %q", res.OIDCConfigError) + } + if res.JWKS != nil { + t.Fatalf("expected nil JWKS on malformed JSON, got %#v", res.JWKS) + } + if res.JWKSError != "failed to unmarshal JWKS response: invalid character '}' looking for beginning of value" { + t.Fatalf("unexpected JWKSError: %q", res.JWKSError) + } +} From 2a994be56d23a5a56afbeaf5b7e9e33df4c4e371 Mon Sep 17 00:00:00 2001 From: Tim Ramlot <42113979+inteon@users.noreply.github.com> Date: Fri, 23 Jan 2026 12:10:01 +0100 Subject: [PATCH 2/2] add comments requested in code review Signed-off-by: Tim Ramlot <42113979+inteon@users.noreply.github.com> --- pkg/datagatherer/oidc/oidc.go | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/pkg/datagatherer/oidc/oidc.go b/pkg/datagatherer/oidc/oidc.go index d68fbfc9..069c482a 100644 --- a/pkg/datagatherer/oidc/oidc.go +++ b/pkg/datagatherer/oidc/oidc.go @@ -48,6 +48,8 @@ type DataGathererOIDC struct { cl rest.Interface } +var _ datagatherer.DataGatherer = &DataGathererOIDC{} + func (g *DataGathererOIDC) Run(ctx context.Context) error { return nil } @@ -57,7 +59,7 @@ func (g *DataGathererOIDC) WaitForCacheSync(ctx context.Context) error { return nil } -// Fetch will fetch discovery data from the apiserver, or return an error +// Fetch will fetch the OIDC discovery document and JWKS from the cluster API server. func (g *DataGathererOIDC) Fetch() (any, int, error) { ctx := context.Background() @@ -76,7 +78,7 @@ func (g *DataGathererOIDC) Fetch() (any, int, error) { OIDCConfigError: errToString(oidcErr), JWKS: jwksResponse, JWKSError: errToString(jwksErr), - }, 1, nil + }, 1 /* we have 1 result, so return 1 as count */, nil } type OIDCDiscoveryData struct { @@ -87,6 +89,7 @@ type OIDCDiscoveryData struct { } func (g *DataGathererOIDC) fetchOIDCConfig(ctx context.Context) (map[string]any, error) { + // Fetch the OIDC discovery document from the well-known endpoint. bytes, err := g.cl.Get().AbsPath("/.well-known/openid-configuration").Do(ctx).Raw() if err != nil { 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, } func (g *DataGathererOIDC) fetchJWKS(ctx context.Context) (map[string]any, error) { + // Fetch the JWKS from the default /openid/v1/jwks endpoint. + // We are not using the jwks_uri from the OIDC config because: + // - on hybrid OpenShift clusters, we saw it pointed to a non-existent URL + // - on fully private AWS EKS clusters, the URL is still public and might not + // be reachable from within the cluster (https://github.com/aws/containers-roadmap/issues/2038) + // So we are using the default path instead, which we think should work in most cases. bytes, err := g.cl.Get().AbsPath("/openid/v1/jwks").Do(ctx).Raw() if err != nil { return nil, fmt.Errorf("failed to get JWKS from jwks_uri: %v", err)