From d5a90d60f97834fde8431b9535499e936b810373 Mon Sep 17 00:00:00 2001 From: David Simansky Date: Tue, 1 Sep 2026 13:22:33 +0200 Subject: [PATCH] feat: reduce deploy SA token expiry to 30 days The deploy ServiceAccount token was minted with a 365 day (1 year) lifetime and stored in a GitHub Actions secret. A credential that long-lived is unnecessary exposure if the secret leaks, since it far outlasts the interval between deployments. Reduce the default lifetime to 30 days and make it configurable via the SA_TOKEN_EXPIRY env var, accepting common duration notation such as 30d, 10h, or 7d12h. The value is wired through a Helm value (plugin.saTokenExpiry) into the deployment's container env. The token is still a one-shot credential with no in-product refresh, so a function's CI stops deploying once the token expires. A user-triggered re-issue path and keyless rotation are tracked as follow-ups. Co-Authored-By: Claude Opus 4.8 (1M context) --- backend/cluster/client.go | 70 ++++++++++++++++--- backend/cluster/client_test.go | 56 +++++++++++++++ backend/handler/common_test.go | 2 +- backend/handler/create.go | 2 +- backend/handler/create_test.go | 2 +- backend/handler/handler.go | 5 +- backend/main.go | 8 ++- .../templates/deployment.yaml | 5 ++ charts/openshift-console-plugin/values.yaml | 3 + 9 files changed, 138 insertions(+), 15 deletions(-) diff --git a/backend/cluster/client.go b/backend/cluster/client.go index 326799b7..18deef43 100644 --- a/backend/cluster/client.go +++ b/backend/cluster/client.go @@ -2,8 +2,12 @@ package cluster import ( "context" + "errors" "fmt" "log/slog" + "strconv" + "strings" + "time" "github.com/openshift/faas-console-plugin/backend/kube" authenticationv1 "k8s.io/api/authentication/v1" @@ -31,15 +35,62 @@ type Client interface { RequestToken(ctx context.Context, namespace string) (string, error) } -// DefaultTokenExpiry is the requested SA token lifetime in seconds. Matches the -// previous frontend behaviour. Security concern: a long-lived token in an SCM -// Actions secret increases exposure if leaked; shorter expiry is a follow-up. -const DefaultTokenExpiry int64 = 365 * 24 * 60 * 60 // 1 year +// DefaultTokenExpiry is the requested SA token lifetime in seconds when no +// override is configured. Kept short to limit exposure of the token stored in +// an SCM Actions secret if leaked. Override via the SA_TOKEN_EXPIRY env var +// (a duration such as 30d, 10h, or 7d12h); see ParseTokenExpiry. +const DefaultTokenExpiry int64 = 30 * 24 * 60 * 60 // 30 days -// New creates a cluster client authenticated with token. +// errExpiryFormat describes the accepted SA_TOKEN_EXPIRY notation. +var errExpiryFormat = errors.New("must be a duration such as 30d, 10h, or 7d12h") + +// ParseTokenExpiry converts the SA_TOKEN_EXPIRY value into a token lifetime in +// seconds. The value is a duration in common notation, e.g. 30d, 10h, or +// 7d12h. The 'd' (days) unit extends Go's standard duration units (h, m, s). +// An empty value yields DefaultTokenExpiry. +func ParseTokenExpiry(s string) (int64, error) { + if s == "" { + return DefaultTokenExpiry, nil + } + d, err := parseExpiryDuration(s) + if err != nil { + return 0, fmt.Errorf("invalid token expiry %q: %w", s, err) + } + secs := int64(d / time.Second) + if secs <= 0 { + return 0, fmt.Errorf("invalid token expiry %q: must be at least one second", s) + } + return secs, nil +} + +// parseExpiryDuration parses a duration that may include a leading days +// component (e.g. 7d or 7d12h). time.ParseDuration handles h/m/s but not d. +func parseExpiryDuration(s string) (time.Duration, error) { + var total time.Duration + rest := s + if i := strings.IndexByte(rest, 'd'); i >= 0 { + days, err := strconv.Atoi(rest[:i]) + if err != nil { + return 0, errExpiryFormat + } + total += time.Duration(days) * 24 * time.Hour + rest = rest[i+1:] + } + if rest != "" { + d, err := time.ParseDuration(rest) + if err != nil { + return 0, errExpiryFormat + } + total += d + } + return total, nil +} + +// New creates a cluster client authenticated with token. tokenExpiry is the +// requested SA token lifetime in seconds (see ParseTokenExpiry). // When host is non-empty (dev/test) it is used as the API server URL directly. // When host is empty the standard in-cluster config is used (pod env vars + SA files). -func New(host, token string, caCert []byte) (Client, error) { +func New(host, token string, caCert []byte, tokenExpiry int64) (Client, error) { cfg, err := kube.RESTConfig(host, token, caCert) if err != nil { return nil, err @@ -49,11 +100,12 @@ func New(host, token string, caCert []byte) (Client, error) { if err != nil { return nil, fmt.Errorf("create kubernetes client: %w", err) } - return &k8sClient{clientset: clientset}, nil + return &k8sClient{clientset: clientset, tokenExpiry: tokenExpiry}, nil } type k8sClient struct { - clientset kubernetes.Interface + clientset kubernetes.Interface + tokenExpiry int64 // requested SA token lifetime in seconds } func (c *k8sClient) CreateServiceAccount(ctx context.Context, namespace string) (bool, error) { @@ -181,7 +233,7 @@ func (c *k8sClient) DeleteImageBuilderBinding(ctx context.Context, namespace str } func (c *k8sClient) RequestToken(ctx context.Context, namespace string) (string, error) { - expiry := DefaultTokenExpiry + expiry := c.tokenExpiry result, err := c.clientset.CoreV1().ServiceAccounts(namespace).CreateToken(ctx, saName, &authenticationv1.TokenRequest{ Spec: authenticationv1.TokenRequestSpec{ ExpirationSeconds: &expiry, diff --git a/backend/cluster/client_test.go b/backend/cluster/client_test.go index 625db2aa..995903de 100644 --- a/backend/cluster/client_test.go +++ b/backend/cluster/client_test.go @@ -369,6 +369,28 @@ var _ = Describe("Kubernetes cluster client", func() { Expect(token).To(Equal("sa-token-value")) }) + It("requests a token with the configured expiry", func() { + var requestedExpiry *int64 + cs := fake.NewSimpleClientset() + cs.PrependReactor("create", "serviceaccounts", func(action k8stesting.Action) (bool, runtime.Object, error) { + if action.GetSubresource() != "token" { + return false, nil, nil + } + tr := action.(k8stesting.CreateAction).GetObject().(*authenticationv1.TokenRequest) + requestedExpiry = tr.Spec.ExpirationSeconds + return true, &authenticationv1.TokenRequest{ + Status: authenticationv1.TokenRequestStatus{Token: "sa-token-value"}, + }, nil + }) + cl := &k8sClient{clientset: cs, tokenExpiry: 7 * 24 * 60 * 60} + + _, err := cl.RequestToken(context.Background(), "default") + + Expect(err).NotTo(HaveOccurred()) + Expect(requestedExpiry).NotTo(BeNil()) + Expect(*requestedExpiry).To(Equal(int64(7 * 24 * 60 * 60))) + }) + It("returns an error when the token endpoint is unavailable", func() { cs := fake.NewSimpleClientset() cs.PrependReactor("create", "serviceaccounts", func(action k8stesting.Action) (bool, runtime.Object, error) { @@ -385,3 +407,37 @@ var _ = Describe("Kubernetes cluster client", func() { }) }) }) + +var _ = Describe("ParseTokenExpiry", func() { + It("defaults to 30 days when unset", func() { + secs, err := ParseTokenExpiry("") + + Expect(err).NotTo(HaveOccurred()) + Expect(secs).To(Equal(int64(30 * 24 * 60 * 60))) + }) + + DescribeTable("parses common duration notation", + func(value string, want int64) { + secs, err := ParseTokenExpiry(value) + Expect(err).NotTo(HaveOccurred()) + Expect(secs).To(Equal(want)) + }, + Entry("days", "7d", int64(7*24*60*60)), + Entry("hours", "10h", int64(10*60*60)), + Entry("minutes", "90m", int64(90*60)), + Entry("days and hours combined", "7d12h", int64(7*24*60*60+12*60*60)), + ) + + DescribeTable("rejects invalid values", + func(value string) { + _, err := ParseTokenExpiry(value) + Expect(err).To(HaveOccurred()) + }, + Entry("non-numeric", "banana"), + Entry("missing unit", "720"), + Entry("bare days unit", "d"), + Entry("zero", "0h"), + Entry("negative", "-5h"), + Entry("rounds down to zero seconds", "500ms"), + ) +}) diff --git a/backend/handler/common_test.go b/backend/handler/common_test.go index 247a2aec..aff0aba1 100644 --- a/backend/handler/common_test.go +++ b/backend/handler/common_test.go @@ -19,7 +19,7 @@ func withSCMStub(stub scm.Client) { func withClusterStub(stub cluster.Client) { orig := newClusterClient - newClusterClient = func(host, token string, caCert []byte) (cluster.Client, error) { + newClusterClient = func(host, token string, caCert []byte, tokenExpiry int64) (cluster.Client, error) { return stub, nil } DeferCleanup(func() { newClusterClient = orig }) diff --git a/backend/handler/create.go b/backend/handler/create.go index 94d1a72a..8dca5089 100644 --- a/backend/handler/create.go +++ b/backend/handler/create.go @@ -99,7 +99,7 @@ func (h *Handlers) createFunction(ctx context.Context, req createRequest, pat, o return fmt.Errorf("generate scaffold: %w", err) } - cl, err := newClusterClient(h.kubeHost, ocpToken, h.caCert) + cl, err := newClusterClient(h.kubeHost, ocpToken, h.caCert, h.tokenExpiry) if err != nil { return fmt.Errorf("%w: %w", errUpstream, fmt.Errorf("connect to cluster: %w", err)) } diff --git a/backend/handler/create_test.go b/backend/handler/create_test.go index f08a2d22..8eab5699 100644 --- a/backend/handler/create_test.go +++ b/backend/handler/create_test.go @@ -89,7 +89,7 @@ var _ = Describe("POST /api/v1/func/create", func() { Entry("cluster client connection fails", func() { orig := newClusterClient - newClusterClient = func(host, token string, caCert []byte) (cluster.Client, error) { + newClusterClient = func(host, token string, caCert []byte, tokenExpiry int64) (cluster.Client, error) { return nil, errors.New("connection refused") } DeferCleanup(func() { newClusterClient = orig }) diff --git a/backend/handler/handler.go b/backend/handler/handler.go index 87f99f23..a8f2856f 100644 --- a/backend/handler/handler.go +++ b/backend/handler/handler.go @@ -13,9 +13,10 @@ type Handlers struct { caCert []byte // cluster CA certificate, read once at startup kubeHost string // API server URL for dev/test; empty uses in-cluster config externalAPIServerURL string // external URL embedded in generated kubeconfigs + tokenExpiry int64 // requested SA token lifetime in seconds } -func New(caPath, kubeHost, externalAPIServerURL string) (*Handlers, error) { +func New(caPath, kubeHost, externalAPIServerURL string, tokenExpiry int64) (*Handlers, error) { var caCert []byte if caPath != "" { var err error @@ -24,7 +25,7 @@ func New(caPath, kubeHost, externalAPIServerURL string) (*Handlers, error) { return nil, fmt.Errorf("read CA certificate %q: %w", caPath, err) } } - return &Handlers{caCert: caCert, kubeHost: kubeHost, externalAPIServerURL: externalAPIServerURL}, nil + return &Handlers{caCert: caCert, kubeHost: kubeHost, externalAPIServerURL: externalAPIServerURL, tokenExpiry: tokenExpiry}, nil } func extractSCMToken(r *http.Request) (string, bool) { diff --git a/backend/main.go b/backend/main.go index 4b954970..34d5df4a 100644 --- a/backend/main.go +++ b/backend/main.go @@ -12,6 +12,7 @@ import ( "net/http" "os" + "github.com/openshift/faas-console-plugin/backend/cluster" "github.com/openshift/faas-console-plugin/backend/config" "github.com/openshift/faas-console-plugin/backend/handler" "github.com/openshift/faas-console-plugin/backend/scm" @@ -48,12 +49,17 @@ func main() { log.Fatal("--external-api-server-url is required") } + tokenExpiry, err := cluster.ParseTokenExpiry(os.Getenv("SA_TOKEN_EXPIRY")) + if err != nil { + log.Fatal(err) + } + static, err := fs.Sub(staticFiles, "static") if err != nil { log.Fatalf("Failed to create sub filesystem: %v", err) } - h, err := handler.New(*caPath, *kubeHost, *kubeAPIServer) + h, err := handler.New(*caPath, *kubeHost, *kubeAPIServer, tokenExpiry) if err != nil { log.Fatal(err) } diff --git a/charts/openshift-console-plugin/templates/deployment.yaml b/charts/openshift-console-plugin/templates/deployment.yaml index d70c3a3d..bb51c20a 100644 --- a/charts/openshift-console-plugin/templates/deployment.yaml +++ b/charts/openshift-console-plugin/templates/deployment.yaml @@ -40,6 +40,11 @@ spec: {{- if .Values.plugin.ghApiUrl }} - "--gh-api-url={{ .Values.plugin.ghApiUrl }}" {{- end }} + {{- if .Values.plugin.saTokenExpiry }} + env: + - name: SA_TOKEN_EXPIRY + value: "{{ .Values.plugin.saTokenExpiry }}" + {{- end }} livenessProbe: httpGet: path: /healthz diff --git a/charts/openshift-console-plugin/values.yaml b/charts/openshift-console-plugin/values.yaml index 76824e91..6ab483f4 100644 --- a/charts/openshift-console-plugin/values.yaml +++ b/charts/openshift-console-plugin/values.yaml @@ -5,6 +5,9 @@ plugin: image: "" apiServerURL: "" ghApiUrl: "" + # saTokenExpiry is the deploy ServiceAccount token lifetime as a duration, + # e.g. 30d, 10h, or 7d12h. Empty uses the backend default (30 days). + saTokenExpiry: "" imagePullPolicy: IfNotPresent imagePullSecrets: [] replicas: 2