From f56ef7eabc385b9112c4168a8ee8067a0ef42c07 Mon Sep 17 00:00:00 2001 From: David Simansky Date: Tue, 15 Sep 2026 20:20:57 +0200 Subject: [PATCH] feat: refresh deploy credentials Deploy ServiceAccount credentials need a recovery path when their short lifetime ends, otherwise CI deployments fail until users manually recreate them. Make token lifetime configurable and refresh the kubeconfig before file updates when the stored credential is within 24 hours of expiry. Persist the refreshed token and expiration so subsequent deployments continue without manual re-issuance. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- backend/cluster/client.go | 24 +- backend/cluster/client_test.go | 28 ++- backend/cluster/kubeconfig.go | 8 +- backend/cluster/kubeconfig_test.go | 28 +-- backend/config/expiry.go | 9 + backend/config/utils.go | 52 ++++ backend/config/utils_test.go | 64 +++++ backend/handler/create.go | 20 +- backend/handler/create_test.go | 29 ++- backend/handler/files.go | 108 ++++++++- backend/handler/files_refresh_test.go | 226 ++++++++++++++++++ backend/handler/files_test.go | 27 ++- backend/handler/handler.go | 24 +- backend/main.go | 9 +- .../templates/deployment.yaml | 3 + charts/openshift-console-plugin/values.yaml | 3 + docs/ARCHITECTURE.md | 11 +- 17 files changed, 605 insertions(+), 68 deletions(-) create mode 100644 backend/config/expiry.go create mode 100644 backend/config/utils.go create mode 100644 backend/config/utils_test.go create mode 100644 backend/handler/files_refresh_test.go diff --git a/backend/cluster/client.go b/backend/cluster/client.go index 326799b7..31e09c82 100644 --- a/backend/cluster/client.go +++ b/backend/cluster/client.go @@ -28,14 +28,9 @@ type Client interface { DeleteRoleBinding(ctx context.Context, namespace string) error CreateImageBuilderBinding(ctx context.Context, namespace string) (bool, error) DeleteImageBuilderBinding(ctx context.Context, namespace string) error - RequestToken(ctx context.Context, namespace string) (string, error) + RequestToken(ctx context.Context, namespace string, saTokenExpirty int64) (*authenticationv1.TokenRequestStatus, 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 - // New creates a cluster client authenticated with token. // 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). @@ -180,18 +175,17 @@ func (c *k8sClient) DeleteImageBuilderBinding(ctx context.Context, namespace str return nil } -func (c *k8sClient) RequestToken(ctx context.Context, namespace string) (string, error) { - expiry := DefaultTokenExpiry +func (c *k8sClient) RequestToken(ctx context.Context, namespace string, saTokenExpiry int64) (*authenticationv1.TokenRequestStatus, error) { result, err := c.clientset.CoreV1().ServiceAccounts(namespace).CreateToken(ctx, saName, &authenticationv1.TokenRequest{ Spec: authenticationv1.TokenRequestSpec{ - ExpirationSeconds: &expiry, + ExpirationSeconds: &saTokenExpiry, }, }, metav1.CreateOptions{}) if err != nil { - return "", fmt.Errorf("request token: %w", err) + return nil, fmt.Errorf("request token: %w", err) } slog.Info("service account token issued", "namespace", namespace, "expires", result.Status.ExpirationTimestamp) - return result.Status.Token, nil + return &result.Status, nil } type ClientStub struct { @@ -199,7 +193,7 @@ type ClientStub struct { OnApplyRole func(ctx context.Context, namespace string) (bool, error) OnCreateRoleBinding func(ctx context.Context, namespace string) (bool, error) OnCreateImageBuilderBinding func(ctx context.Context, namespace string) (bool, error) - OnRequestToken func(ctx context.Context, namespace string) (string, error) + OnRequestToken func(ctx context.Context, namespace string, saTokenExpiry int64) (*authenticationv1.TokenRequestStatus, error) OnDeleteServiceAccount func(ctx context.Context, namespace string) error OnDeleteRole func(ctx context.Context, namespace string) error OnDeleteRoleBinding func(ctx context.Context, namespace string) error @@ -234,11 +228,11 @@ func (s *ClientStub) CreateImageBuilderBinding(ctx context.Context, namespace st return true, nil } -func (s *ClientStub) RequestToken(ctx context.Context, namespace string) (string, error) { +func (s *ClientStub) RequestToken(ctx context.Context, namespace string, saTokenExpiry int64) (*authenticationv1.TokenRequestStatus, error) { if s.OnRequestToken != nil { - return s.OnRequestToken(ctx, namespace) + return s.OnRequestToken(ctx, namespace, saTokenExpiry) } - return "stub-token", nil + return &authenticationv1.TokenRequestStatus{Token: "stub-token"}, nil } func (s *ClientStub) DeleteServiceAccount(ctx context.Context, namespace string) error { diff --git a/backend/cluster/client_test.go b/backend/cluster/client_test.go index 625db2aa..ecfc7d6d 100644 --- a/backend/cluster/client_test.go +++ b/backend/cluster/client_test.go @@ -363,10 +363,32 @@ var _ = Describe("Kubernetes cluster client", func() { }) cl := &k8sClient{clientset: cs} - token, err := cl.RequestToken(context.Background(), "default") + tokenStatus, err := cl.RequestToken(context.Background(), "default", 30*24*60*60) Expect(err).NotTo(HaveOccurred()) - Expect(token).To(Equal("sa-token-value")) + Expect(tokenStatus.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} + + _, err := cl.RequestToken(context.Background(), "default", 7*24*60*60) + + 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() { @@ -379,7 +401,7 @@ var _ = Describe("Kubernetes cluster client", func() { }) cl := &k8sClient{clientset: cs} - _, err := cl.RequestToken(context.Background(), "default") + _, err := cl.RequestToken(context.Background(), "default", 30*24*60*60) Expect(err).To(HaveOccurred()) }) diff --git a/backend/cluster/kubeconfig.go b/backend/cluster/kubeconfig.go index 516b078e..66bf5cfd 100644 --- a/backend/cluster/kubeconfig.go +++ b/backend/cluster/kubeconfig.go @@ -1,23 +1,17 @@ package cluster import ( - "context" "fmt" "k8s.io/client-go/tools/clientcmd" clientcmdapi "k8s.io/client-go/tools/clientcmd/api" ) -func GenerateKubeconfig(ctx context.Context, client Client, namespace, externalAPIServerURL string, caCert []byte) (string, error) { +func GenerateKubeconfig(namespace, externalAPIServerURL, token string, caCert []byte) (string, error) { if externalAPIServerURL == "" { return "", fmt.Errorf("API server URL is required") } - token, err := client.RequestToken(ctx, namespace) - if err != nil { - return "", fmt.Errorf("request token: %w", err) - } - return buildKubeconfig(externalAPIServerURL, token, namespace, caCert) } diff --git a/backend/cluster/kubeconfig_test.go b/backend/cluster/kubeconfig_test.go index 364ee804..7c8b51bf 100644 --- a/backend/cluster/kubeconfig_test.go +++ b/backend/cluster/kubeconfig_test.go @@ -44,7 +44,10 @@ var _ = Describe("GenerateKubeconfig", func() { It("returns a valid kubeconfig with the token and server URL", func() { cl, _ := fullFakeClient("sa-token-value") - kubeconfig, err := GenerateKubeconfig(context.Background(), cl, "default", fakeAPIURL, nil) + tokenStatus, err := cl.RequestToken(context.Background(), "default", 30*24*60*60) + Expect(err).NotTo(HaveOccurred()) + + kubeconfig, err := GenerateKubeconfig("default", fakeAPIURL, tokenStatus.Token, nil) Expect(err).NotTo(HaveOccurred()) @@ -61,10 +64,9 @@ var _ = Describe("GenerateKubeconfig", func() { }) It("embeds the CA certificate when the cluster uses a private CA", func() { - cl, _ := fullFakeClient("sa-token-value") caCert := []byte("-----BEGIN CERTIFICATE-----\nfake\n-----END CERTIFICATE-----\n") - kubeconfig, err := GenerateKubeconfig(context.Background(), cl, "default", fakeAPIURL, caCert) + kubeconfig, err := GenerateKubeconfig("default", fakeAPIURL, "sa-token-value", caCert) Expect(err).NotTo(HaveOccurred()) var parsed map[string]any @@ -74,26 +76,8 @@ var _ = Describe("GenerateKubeconfig", func() { Expect(cluster).To(HaveKey("certificate-authority-data")) }) - It("returns an error when requesting the service account token fails", func() { - cs := fake.NewSimpleClientset() - cs.PrependReactor("create", "serviceaccounts", func(action k8stesting.Action) (bool, runtime.Object, error) { - if action.GetSubresource() == "token" { - return true, nil, forbiddenFor("serviceaccounts") - } - return false, nil, nil - }) - cl := &k8sClient{clientset: cs} - - _, err := GenerateKubeconfig(context.Background(), cl, "default", fakeAPIURL, nil) - - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("request token")) - }) - It("returns an error when the external API server URL is empty", func() { - cl, _ := fullFakeClient("sa-token-value") - - _, err := GenerateKubeconfig(context.Background(), cl, "default", "", nil) + _, err := GenerateKubeconfig("default", "", "sa-token-value", nil) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("API server URL is required")) diff --git a/backend/config/expiry.go b/backend/config/expiry.go new file mode 100644 index 00000000..d9f5725e --- /dev/null +++ b/backend/config/expiry.go @@ -0,0 +1,9 @@ +package config + +import "time" + +// DefaultSATokenExpiry is requested duration of validity of the requested ServiceAccount token +const DefaultSATokenExpiry int64 = 7 * 24 * 60 * 60 // 7 days + +// TokenRefreshWindow is the remaining token lifetime at which credentials should be refreshed. +const TokenRefreshWindow = 24 * time.Hour diff --git a/backend/config/utils.go b/backend/config/utils.go new file mode 100644 index 00000000..053b075a --- /dev/null +++ b/backend/config/utils.go @@ -0,0 +1,52 @@ +package config + +import ( + "errors" + "fmt" + "strconv" + "strings" + "time" +) + +// ParseSATokenExpiry converts the DEFAULT_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 defaultSATokenExpiry. +func ParseSATokenExpiry(s string) (int64, error) { + if s == "" { + return DefaultSATokenExpiry, 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 + var errExpiryFormat = errors.New("must be a duration such as 30d, 10h, or 7d12h") + 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 +} diff --git a/backend/config/utils_test.go b/backend/config/utils_test.go new file mode 100644 index 00000000..7e6eb568 --- /dev/null +++ b/backend/config/utils_test.go @@ -0,0 +1,64 @@ +package config + +import ( + "testing" + "time" +) + +func TestParseSATokenExpiry(t *testing.T) { + tests := []struct { + name string + value string + want int64 + wantErr bool + }{ + {name: "empty value uses default", value: "", want: DefaultSATokenExpiry}, + {name: "days", value: "30d", want: int64(30 * 24 * 60 * 60)}, + {name: "hours", value: "10h", want: int64(10 * 60 * 60)}, + {name: "days and hours", value: "7d12h", want: int64(7*24*60*60 + 12*60*60)}, + {name: "hours and minutes", value: "1h30m", want: int64(90 * 60)}, + {name: "malformed value", value: "not-a-duration", wantErr: true}, + {name: "invalid days component", value: "days1h", wantErr: true}, + {name: "invalid duration component", value: "1d2d", wantErr: true}, + {name: "zero", value: "0s", wantErr: true}, + {name: "negative", value: "-1s", wantErr: true}, + {name: "less than one second", value: "500ms", wantErr: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := ParseSATokenExpiry(tt.value) + if (err != nil) != tt.wantErr { + t.Fatalf("ParseSATokenExpiry(%q) error = %v, wantErr %v", tt.value, err, tt.wantErr) + } + if got != tt.want { + t.Fatalf("ParseSATokenExpiry(%q) = %d, want %d", tt.value, got, tt.want) + } + }) + } +} + +func TestParseExpiryDuration(t *testing.T) { + tests := []struct { + name string + value string + want time.Duration + wantErr bool + }{ + {name: "days only", value: "2d", want: 48 * time.Hour}, + {name: "non-numeric days", value: "days1h", wantErr: true}, + {name: "invalid standard duration", value: "1d2d", wantErr: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := parseExpiryDuration(tt.value) + if (err != nil) != tt.wantErr { + t.Fatalf("parseExpiryDuration(%q) error = %v, wantErr %v", tt.value, err, tt.wantErr) + } + if got != tt.want { + t.Fatalf("parseExpiryDuration(%q) = %s, want %s", tt.value, got, tt.want) + } + }) + } +} diff --git a/backend/handler/create.go b/backend/handler/create.go index 28eee07a..46cafc7e 100644 --- a/backend/handler/create.go +++ b/backend/handler/create.go @@ -9,6 +9,7 @@ import ( "net/http" "regexp" "strings" + "time" "github.com/openshift/faas-console-plugin/backend/cluster" "github.com/openshift/faas-console-plugin/backend/config" @@ -18,8 +19,9 @@ import ( ) const ( - repoSecretKubeconfig = "KUBECONFIG" - repoVarClusterAPIURL = "CLUSTER_API_URL" + repoSecretKubeconfig = "KUBECONFIG" + repoKubeconfigExpireAt = "KUBECONFIG_EXPIRE_AT" + repoVarClusterAPIURL = "CLUSTER_API_URL" ) var ( @@ -119,7 +121,12 @@ func (h *Handlers) createFunction(ctx context.Context, req createRequest, pat, o } }() - kubeconfig, err := cluster.GenerateKubeconfig(ctx, cl, req.Namespace, h.externalAPIServerURL, h.caCert) + tokenStatus, err := cl.RequestToken(ctx, req.Namespace, h.saTokenExpiry) + if err != nil { + return fmt.Errorf("%w: %w", errUpstream, fmt.Errorf("request token: %w", err)) + } + + kubeconfig, err := cluster.GenerateKubeconfig(req.Namespace, h.externalAPIServerURL, tokenStatus.Token, h.caCert) if err != nil { return fmt.Errorf("%w: %w", errUpstream, fmt.Errorf("generate kubeconfig: %w", err)) } @@ -147,6 +154,13 @@ func (h *Handlers) createFunction(ctx context.Context, req createRequest, pat, o slog.Error("failed to store CI secret", "owner", req.Owner, "repo", req.Repo, "err", err) return fmt.Errorf("%w: %w", errUpstream, fmt.Errorf("store secret: %w", err)) } + if err := client.StoreVariable(ctx, req.Owner, req.Repo, repoKubeconfigExpireAt, tokenStatus.ExpirationTimestamp.Time.UTC().Format(time.RFC3339)); err != nil { + if errors.Is(err, scm.ErrUnauthorized) { + return err + } + slog.Error("failed to store EXPIRE_AT variable", "owner", req.Owner, "repo", req.Repo, "err", err) + return fmt.Errorf("%w: %w", errUpstream, fmt.Errorf("store variable: %w", err)) + } if err := client.StoreVariable(ctx, req.Owner, req.Repo, repoVarClusterAPIURL, h.externalAPIServerURL); err != nil { if errors.Is(err, scm.ErrUnauthorized) { return err diff --git a/backend/handler/create_test.go b/backend/handler/create_test.go index c5839e64..2c986b0b 100644 --- a/backend/handler/create_test.go +++ b/backend/handler/create_test.go @@ -14,6 +14,7 @@ import ( "github.com/openshift/faas-console-plugin/backend/cluster" "github.com/openshift/faas-console-plugin/backend/functions" "github.com/openshift/faas-console-plugin/backend/scm" + authenticationv1 "k8s.io/api/authentication/v1" ) var _ = Describe("POST /api/v1/func/create", func() { @@ -74,6 +75,28 @@ var _ = Describe("POST /api/v1/func/create", func() { Expect(gotPushFiles).NotTo(BeEmpty()) }) + It("uses the configured service account token expiry", func() { + var requestedExpiry int64 + withSCMStub(&scm.ClientStub{}) + withClusterStub(&cluster.ClientStub{ + OnRequestToken: func(ctx context.Context, namespace string, saTokenExpiry int64) (*authenticationv1.TokenRequestStatus, error) { + requestedExpiry = saTokenExpiry + return &authenticationv1.TokenRequestStatus{Token: "stub-token"}, nil + }, + }) + h, err := New("", "", "https://api.test-cluster.example.com:6443", 7*24*60*60) + Expect(err).NotTo(HaveOccurred()) + req := httptest.NewRequest(http.MethodPost, "/api/v1/func/create", bytes.NewBuffer(validBody())) + req.Header.Set("X-SCM-Token", "test-pat") + req.Header.Set("Authorization", "Bearer ocp-token") + w := httptest.NewRecorder() + + h.HandleFuncCreate(w, req) + + Expect(w.Code).To(Equal(http.StatusCreated)) + Expect(requestedExpiry).To(Equal(int64(7 * 24 * 60 * 60))) + }) + DescribeTable("maps upstream errors to HTTP status codes", func(setup func(), expectedCode int) { w := doCreate(setup) @@ -243,7 +266,7 @@ var _ = Describe("POST /api/v1/func/create", func() { ) Describe("rollback on failure", func() { - It("rolls back cluster resources when GenerateKubeconfig fails", func() { + It("rolls back cluster resources when requesting the service account token fails", func() { calls := map[string]int{} recordCall := func(key string) { calls[key]++ } @@ -255,8 +278,8 @@ var _ = Describe("POST /api/v1/func/create", func() { }, }) withClusterStub(&cluster.ClientStub{ - OnRequestToken: func(ctx context.Context, namespace string) (string, error) { - return "", errors.New("token endpoint unavailable") + OnRequestToken: func(ctx context.Context, namespace string, saTokenExpiry int64) (*authenticationv1.TokenRequestStatus, error) { + return nil, errors.New("token endpoint unavailable") }, OnDeleteServiceAccount: func(ctx context.Context, namespace string) error { recordCall("deleteServiceAccount") diff --git a/backend/handler/files.go b/backend/handler/files.go index 218c2078..838a7eae 100644 --- a/backend/handler/files.go +++ b/backend/handler/files.go @@ -7,9 +7,12 @@ import ( "net/http" "regexp" "strings" + "time" + "github.com/openshift/faas-console-plugin/backend/cluster" "github.com/openshift/faas-console-plugin/backend/config" "github.com/openshift/faas-console-plugin/backend/scm" + k8svalidation "k8s.io/apimachinery/pkg/util/validation" ) var validGitRef = regexp.MustCompile(`^[a-zA-Z0-9]([a-zA-Z0-9._/-]*[a-zA-Z0-9])?$`) @@ -59,6 +62,12 @@ type putFilesRequest struct { Branch string `json:"branch"` } +type putFilesTarget struct { + owner string + repo string + branch string +} + func (h *Handlers) HandlePutFiles(w http.ResponseWriter, r *http.Request) { pat, ok := extractSCMToken(r) if !ok { @@ -98,15 +107,110 @@ func (h *Handlers) HandlePutFiles(w http.ResponseWriter, r *http.Request) { } client := config.SCMRegistry.Client(scm.DefaultPlatform, pat) - if err := client.PushFiles(r.Context(), owner, name, req.Branch, req.Message, req.Files); err != nil { + target := putFilesTarget{owner: owner, repo: name, branch: req.Branch} + if err := h.refreshKubeconfig(r, client, target); err != nil { + if responseErr, ok := errors.AsType[*httpError](err); ok { + slog.Error("failed to refresh kubeconfig", "err", err) + writeError(w, responseErr.code, responseErr.message) + return + } + writeError(w, http.StatusInternalServerError, "internal server error") + return + } + + if err := client.PushFiles(r.Context(), target.owner, target.repo, target.branch, req.Message, req.Files); err != nil { if errors.Is(err, scm.ErrUnauthorized) { writeError(w, http.StatusUnauthorized, "invalid SCM token") return } - slog.Error("failed to push files", "owner", owner, "repo", name, "err", err) + slog.Error("failed to push files", "owner", target.owner, "repo", target.repo, "err", err) writeError(w, http.StatusBadGateway, "failed to push files to repository") return } w.WriteHeader(http.StatusNoContent) } + +func (h *Handlers) refreshKubeconfig(r *http.Request, client scm.Client, target putFilesTarget) error { + expiration, err := client.GetVariable(r.Context(), target.owner, target.repo, repoKubeconfigExpireAt) + if err != nil { + if errors.Is(err, scm.ErrUnauthorized) { + return newHTTPError(http.StatusUnauthorized, "invalid SCM token", err) + } + slog.Error("failed to read deployment credential expiration", "owner", target.owner, "repo", target.repo, "err", err) + return newHTTPError(http.StatusBadGateway, "failed to check deployment credentials", err) + } + + if !tokenNeedsRefresh(expiration, time.Now()) { + return nil + } + + ocpToken, ok := extractOCPToken(r) + if !ok { + return newHTTPError(http.StatusUnauthorized, "Authorization header is required", nil) + } + + funcYaml, err := client.GetFileContent(r.Context(), target.owner, target.repo, target.branch, "func.yaml") + if err != nil { + if errors.Is(err, scm.ErrUnauthorized) { + return newHTTPError(http.StatusUnauthorized, "invalid SCM token", err) + } + slog.Error("failed to read func.yaml", "owner", target.owner, "repo", target.repo, "branch", target.branch, "err", err) + return newHTTPError(http.StatusBadGateway, "failed to read function configuration", err) + } + _, namespace, _, err := parseFuncYaml(funcYaml) + if err != nil { + slog.Error("failed to parse func.yaml", "owner", target.owner, "repo", target.repo, "branch", target.branch, "err", err) + return newHTTPError(http.StatusUnprocessableEntity, "invalid function configuration", err) + } + if errs := k8svalidation.IsDNS1123Label(namespace); len(errs) > 0 { + slog.Error("invalid namespace in func.yaml", "owner", target.owner, "repo", target.repo, "branch", target.branch) + return newHTTPError(http.StatusUnprocessableEntity, "invalid namespace in function configuration", errors.New(errs[0])) + } + + clusterClient, err := newClusterClient(h.kubeHost, ocpToken, h.caCert) + if err != nil { + slog.Error("failed to connect to cluster", "namespace", namespace, "err", err) + return newHTTPError(http.StatusBadGateway, "failed to refresh deployment credentials", err) + } + + tokenStatus, err := clusterClient.RequestToken(r.Context(), namespace, h.saTokenExpiry) + if err != nil { + slog.Error("failed to request refreshed service account token", "namespace", namespace, "err", err) + return newHTTPError(http.StatusBadGateway, "failed to refresh deployment credentials", err) + } + + kubeconfig, err := cluster.GenerateKubeconfig(namespace, h.externalAPIServerURL, tokenStatus.Token, h.caCert) + if err != nil { + slog.Error("failed to generate refreshed kubeconfig", "namespace", namespace, "err", err) + return newHTTPError(http.StatusBadGateway, "failed to refresh deployment credentials", err) + } + if err := client.StoreSecret(r.Context(), target.owner, target.repo, repoSecretKubeconfig, kubeconfig); err != nil { + if errors.Is(err, scm.ErrUnauthorized) { + return newHTTPError(http.StatusUnauthorized, "invalid SCM token", err) + } + slog.Error("failed to update CI secret", "owner", target.owner, "repo", target.repo, "err", err) + return newHTTPError(http.StatusBadGateway, "failed to update deployment secret", err) + } + if err := client.StoreVariable(r.Context(), target.owner, target.repo, repoKubeconfigExpireAt, tokenStatus.ExpirationTimestamp.Time.UTC().Format(time.RFC3339)); err != nil { + if errors.Is(err, scm.ErrUnauthorized) { + return newHTTPError(http.StatusUnauthorized, "invalid SCM token", err) + } + slog.Error("failed to update deployment credential expiration", "owner", target.owner, "repo", target.repo, "err", err) + return newHTTPError(http.StatusBadGateway, "failed to update deployment credentials", err) + } + + return nil +} + +func tokenNeedsRefresh(expiration string, now time.Time) bool { + if expiration == "" { + return true + } + + expiresAt, err := time.Parse(time.RFC3339, expiration) + if err != nil { + return true + } + return expiresAt.Sub(now) <= config.TokenRefreshWindow +} diff --git a/backend/handler/files_refresh_test.go b/backend/handler/files_refresh_test.go new file mode 100644 index 00000000..04f2c72f --- /dev/null +++ b/backend/handler/files_refresh_test.go @@ -0,0 +1,226 @@ +package handler + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "net/http" + "net/http/httptest" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/openshift/faas-console-plugin/backend/cluster" + "github.com/openshift/faas-console-plugin/backend/config" + "github.com/openshift/faas-console-plugin/backend/scm" + authenticationv1 "k8s.io/api/authentication/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +var _ = Describe("PUT files deployment credential refresh", func() { + validBody := func() []byte { + body, _ := json.Marshal(putFilesRequest{ + Files: []scm.FileEntry{{Path: "func.go", Mode: "100644", Content: "package main", Type: "blob"}}, + Message: "Update function files", + Branch: "main", + }) + return body + } + newRequest := func() *http.Request { + req := httptest.NewRequest(http.MethodPut, "/api/v1/func/alice/my-func/files", bytes.NewBuffer(validBody())) + req.Header.Set("Authorization", "Bearer ocp-token") + req.Header.Set("X-SCM-Token", "test-pat") + req.SetPathValue("owner", "alice") + req.SetPathValue("name", "my-func") + return req + } + newHandlers := func() *Handlers { + return &Handlers{externalAPIServerURL: "https://api.test-cluster.example.com:6443"} + } + + It("refreshes the deploy kubeconfig before committing the changes", func() { + var calls []string + var gotKubeconfig string + var gotTokenExpiry int64 + expiration := time.Now().Add(12 * time.Hour).UTC().Format(time.RFC3339) + newExpiration := metav1.NewTime(time.Now().Add(7 * 24 * time.Hour)) + withClusterStub(&cluster.ClientStub{ + OnRequestToken: func(ctx context.Context, namespace string, saTokenExpiry int64) (*authenticationv1.TokenRequestStatus, error) { + calls = append(calls, "requestToken") + Expect(namespace).To(Equal("demo")) + gotTokenExpiry = saTokenExpiry + return &authenticationv1.TokenRequestStatus{Token: "fresh-sa-token", ExpirationTimestamp: newExpiration}, nil + }, + }) + withSCMStub(&scm.ClientStub{ + OnGetVariable: func(ctx context.Context, owner, repo, name string) (string, error) { + calls = append(calls, "getExpiration") + Expect(name).To(Equal(repoKubeconfigExpireAt)) + return expiration, nil + }, + OnGetFileContent: func(ctx context.Context, owner, repo, ref, path string) (string, error) { + calls = append(calls, "getFuncYaml") + Expect(ref).To(Equal("main")) + Expect(path).To(Equal("func.yaml")) + return "name: my-func\nnamespace: demo\nruntime: go\n", nil + }, + OnStoreSecret: func(ctx context.Context, owner, repo, name, value string) error { + calls = append(calls, "storeSecret") + Expect(owner).To(Equal("alice")) + Expect(repo).To(Equal("my-func")) + Expect(name).To(Equal("KUBECONFIG")) + gotKubeconfig = value + return nil + }, + OnStoreVariable: func(ctx context.Context, owner, repo, name, value string) error { + calls = append(calls, "storeExpiration") + Expect(name).To(Equal(repoKubeconfigExpireAt)) + Expect(value).To(Equal(newExpiration.Time.UTC().Format(time.RFC3339))) + return nil + }, + OnPushFiles: func(ctx context.Context, owner, repo, branch, message string, files []scm.FileEntry) error { + calls = append(calls, "pushFiles") + return nil + }, + }) + + w := httptest.NewRecorder() + h := newHandlers() + h.saTokenExpiry = 7 * 24 * 60 * 60 + h.HandlePutFiles(w, newRequest()) + + Expect(w.Code).To(Equal(http.StatusNoContent)) + Expect(calls).To(Equal([]string{"getExpiration", "getFuncYaml", "requestToken", "storeSecret", "storeExpiration", "pushFiles"})) + Expect(gotTokenExpiry).To(Equal(int64(7 * 24 * 60 * 60))) + Expect(gotKubeconfig).To(ContainSubstring("fresh-sa-token")) + }) + + It("commits changes without refreshing credentials when they expire in more than 24 hours", func() { + var tokenRequested, filesPushed bool + withClusterStub(&cluster.ClientStub{ + OnRequestToken: func(ctx context.Context, namespace string, saTokenExpiry int64) (*authenticationv1.TokenRequestStatus, error) { + tokenRequested = true + return nil, errors.New("token refresh was not expected") + }, + }) + withSCMStub(&scm.ClientStub{ + OnGetVariable: func(ctx context.Context, owner, repo, name string) (string, error) { + return time.Now().Add(48 * time.Hour).UTC().Format(time.RFC3339), nil + }, + OnPushFiles: func(ctx context.Context, owner, repo, branch, message string, files []scm.FileEntry) error { + filesPushed = true + return nil + }, + }) + w := httptest.NewRecorder() + req := newRequest() + req.Header.Del("Authorization") + + newHandlers().HandlePutFiles(w, req) + + Expect(w.Code).To(Equal(http.StatusNoContent)) + Expect(tokenRequested).To(BeFalse()) + Expect(filesPushed).To(BeTrue()) + }) + + It("rejects requests without an OCP token", func() { + req := newRequest() + req.Header.Del("Authorization") + w := httptest.NewRecorder() + + newHandlers().HandlePutFiles(w, req) + + Expect(w.Code).To(Equal(http.StatusUnauthorized)) + }) + + It("does not update the secret or commit files when token refresh fails", func() { + var secretUpdated, filesPushed bool + withClusterStub(&cluster.ClientStub{ + OnRequestToken: func(ctx context.Context, namespace string, saTokenExpiry int64) (*authenticationv1.TokenRequestStatus, error) { + return nil, errors.New("token endpoint unavailable") + }, + }) + withSCMStub(&scm.ClientStub{ + OnGetFileContent: func(ctx context.Context, owner, repo, ref, path string) (string, error) { + return "name: my-func\nnamespace: demo\nruntime: go\n", nil + }, + OnStoreSecret: func(ctx context.Context, owner, repo, name, value string) error { + secretUpdated = true + return nil + }, + OnPushFiles: func(ctx context.Context, owner, repo, branch, message string, files []scm.FileEntry) error { + filesPushed = true + return nil + }, + }) + w := httptest.NewRecorder() + + newHandlers().HandlePutFiles(w, newRequest()) + + Expect(w.Code).To(Equal(http.StatusBadGateway)) + Expect(secretUpdated).To(BeFalse()) + Expect(filesPushed).To(BeFalse()) + }) + + It("does not commit files when updating the deployment secret fails", func() { + var filesPushed bool + withClusterStub(&cluster.ClientStub{}) + withSCMStub(&scm.ClientStub{ + OnGetFileContent: func(ctx context.Context, owner, repo, ref, path string) (string, error) { + return "name: my-func\nnamespace: demo\nruntime: go\n", nil + }, + OnStoreSecret: func(ctx context.Context, owner, repo, name, value string) error { + return errors.New("github unavailable") + }, + OnPushFiles: func(ctx context.Context, owner, repo, branch, message string, files []scm.FileEntry) error { + filesPushed = true + return nil + }, + }) + w := httptest.NewRecorder() + + newHandlers().HandlePutFiles(w, newRequest()) + + Expect(w.Code).To(Equal(http.StatusBadGateway)) + Expect(filesPushed).To(BeFalse()) + }) + + It("rejects an invalid namespace read from func.yaml", func() { + withSCMStub(&scm.ClientStub{ + OnGetFileContent: func(ctx context.Context, owner, repo, ref, path string) (string, error) { + return "name: my-func\nnamespace: ../other\nruntime: go\n", nil + }, + }) + w := httptest.NewRecorder() + + newHandlers().HandlePutFiles(w, newRequest()) + + Expect(w.Code).To(Equal(http.StatusUnprocessableEntity)) + }) +}) + +var _ = Describe("tokenNeedsRefresh", func() { + now := time.Date(2026, time.January, 15, 12, 0, 0, 0, time.UTC) + + It("refreshes at the 24-hour boundary", func() { + expiration := now.Add(config.TokenRefreshWindow).Format(time.RFC3339) + + Expect(tokenNeedsRefresh(expiration, now)).To(BeTrue()) + }) + + It("does not refresh when expiration is beyond the 24-hour window", func() { + expiration := now.Add(config.TokenRefreshWindow + time.Second).Format(time.RFC3339) + + Expect(tokenNeedsRefresh(expiration, now)).To(BeFalse()) + }) + + DescribeTable("refreshes when expiration cannot be trusted", + func(expiration string) { + Expect(tokenNeedsRefresh(expiration, now)).To(BeTrue()) + }, + Entry("missing", ""), + Entry("malformed", "not-a-timestamp"), + ) +}) diff --git a/backend/handler/files_test.go b/backend/handler/files_test.go index 48d2273a..ff21ccf8 100644 --- a/backend/handler/files_test.go +++ b/backend/handler/files_test.go @@ -11,6 +11,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "github.com/openshift/faas-console-plugin/backend/cluster" "github.com/openshift/faas-console-plugin/backend/scm" ) @@ -153,11 +154,20 @@ var _ = Describe("PUT /api/v1/func/{owner}/{name}/files", func() { }) return body } + withValidPutSCMStub := func(stub *scm.ClientStub) { + if stub.OnGetFileContent == nil { + stub.OnGetFileContent = func(ctx context.Context, owner, repo, ref, path string) (string, error) { + return "name: my-func\nnamespace: demo\nruntime: go\n", nil + } + } + withSCMStub(stub) + } It("commits the changes to the branch", func() { var gotOwner, gotRepo, gotBranch, gotMessage string var gotFiles []scm.FileEntry - withSCMStub(&scm.ClientStub{ + withClusterStub(&cluster.ClientStub{}) + withValidPutSCMStub(&scm.ClientStub{ OnPushFiles: func(ctx context.Context, owner, repo, branch, message string, files []scm.FileEntry) error { gotOwner, gotRepo, gotBranch, gotMessage = owner, repo, branch, message gotFiles = files @@ -166,11 +176,12 @@ var _ = Describe("PUT /api/v1/func/{owner}/{name}/files", func() { }) req := httptest.NewRequest(http.MethodPut, "/api/v1/func/alice/my-func/files", bytes.NewBuffer(validPutBody())) + req.Header.Set("Authorization", "Bearer ocp-token") req.Header.Set("X-SCM-Token", "test-pat") req.SetPathValue("owner", "alice") req.SetPathValue("name", "my-func") w := httptest.NewRecorder() - (&Handlers{}).HandlePutFiles(w, req) + (&Handlers{externalAPIServerURL: "https://api.test-cluster.example.com:6443"}).HandlePutFiles(w, req) Expect(w.Code).To(Equal(http.StatusNoContent)) Expect(gotOwner).To(Equal("alice")) @@ -291,35 +302,39 @@ var _ = Describe("PUT /api/v1/func/{owner}/{name}/files", func() { }) It("returns 401 when the SCM token is invalid", func() { - withSCMStub(&scm.ClientStub{ + withClusterStub(&cluster.ClientStub{}) + withValidPutSCMStub(&scm.ClientStub{ OnPushFiles: func(ctx context.Context, owner, repo, branch, message string, files []scm.FileEntry) error { return scm.ErrUnauthorized }, }) req := httptest.NewRequest(http.MethodPut, "/api/v1/func/alice/my-func/files", bytes.NewBuffer(validPutBody())) + req.Header.Set("Authorization", "Bearer ocp-token") req.Header.Set("X-SCM-Token", "bad-token") req.SetPathValue("owner", "alice") req.SetPathValue("name", "my-func") w := httptest.NewRecorder() - (&Handlers{}).HandlePutFiles(w, req) + (&Handlers{externalAPIServerURL: "https://api.test-cluster.example.com:6443"}).HandlePutFiles(w, req) Expect(w.Code).To(Equal(http.StatusUnauthorized)) }) It("returns 502 when the SCM API is unavailable", func() { - withSCMStub(&scm.ClientStub{ + withClusterStub(&cluster.ClientStub{}) + withValidPutSCMStub(&scm.ClientStub{ OnPushFiles: func(ctx context.Context, owner, repo, branch, message string, files []scm.FileEntry) error { return errors.New("connection refused") }, }) req := httptest.NewRequest(http.MethodPut, "/api/v1/func/alice/my-func/files", bytes.NewBuffer(validPutBody())) + req.Header.Set("Authorization", "Bearer ocp-token") req.Header.Set("X-SCM-Token", "test-pat") req.SetPathValue("owner", "alice") req.SetPathValue("name", "my-func") w := httptest.NewRecorder() - (&Handlers{}).HandlePutFiles(w, req) + (&Handlers{externalAPIServerURL: "https://api.test-cluster.example.com:6443"}).HandlePutFiles(w, req) Expect(w.Code).To(Equal(http.StatusBadGateway)) }) diff --git a/backend/handler/handler.go b/backend/handler/handler.go index 87f99f23..79ed4fd9 100644 --- a/backend/handler/handler.go +++ b/backend/handler/handler.go @@ -13,9 +13,28 @@ 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 + saTokenExpiry int64 // requested SA token lifetime in seconds } -func New(caPath, kubeHost, externalAPIServerURL string) (*Handlers, error) { +type httpError struct { + code int + message string + cause error +} + +func (e *httpError) Error() string { + return e.message +} + +func (e *httpError) Unwrap() error { + return e.cause +} + +func newHTTPError(code int, message string, cause error) error { + return &httpError{code: code, message: message, cause: cause} +} + +func New(caPath, kubeHost, externalAPIServerURL string, saTokenExpiry int64) (*Handlers, error) { var caCert []byte if caPath != "" { var err error @@ -24,7 +43,8 @@ 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, saTokenExpiry: saTokenExpiry}, nil } func extractSCMToken(r *http.Request) (string, bool) { diff --git a/backend/main.go b/backend/main.go index c68be2a1..b9106050 100644 --- a/backend/main.go +++ b/backend/main.go @@ -34,6 +34,7 @@ func main() { kubeHost := flag.String("kube-host", "", "Kubernetes API server URL for dev/test (empty uses in-cluster config)") kubeAPIServer := flag.String("external-api-server-url", "", "external Kubernetes API server URL embedded in generated kubeconfigs") ghAPIURL := flag.String("gh-api-url", "", "GitHub API base URL (for testing with fake server)") + saTokenExpiry := flag.String("sa-token-expiry", "7d", "Token expiry in days, hours or seconds") flag.Parse() if *ghAPIURL != "" { @@ -55,7 +56,13 @@ func main() { log.Fatalf("Failed to create sub filesystem: %v", err) } - h, err := handler.New(*caPath, *kubeHost, *kubeAPIServer) + saTokenExpiryParsed, err := config.ParseSATokenExpiry(*saTokenExpiry) + if err != nil { + log.Printf("Failed to parse --sa-token-expiry-%s: %v, using default", *saTokenExpiry, err) + saTokenExpiryParsed = config.DefaultSATokenExpiry + } + + h, err := handler.New(*caPath, *kubeHost, *kubeAPIServer, saTokenExpiryParsed) 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 328ce917..2e29e373 100644 --- a/charts/openshift-console-plugin/templates/deployment.yaml +++ b/charts/openshift-console-plugin/templates/deployment.yaml @@ -40,6 +40,9 @@ spec: {{- if .Values.plugin.ghApiUrl }} - "--gh-api-url={{ .Values.plugin.ghApiUrl }}" {{- end }} + {{- if .Values.plugin.saTokenExpiry }} + - "--sa-token-expiry={{ .Values.plugin.saTokenExpiry }}" + {{- end }} {{- if .Values.plugin.funcCliVersion }} env: - name: FUNC_CLI_VERSION diff --git a/charts/openshift-console-plugin/values.yaml b/charts/openshift-console-plugin/values.yaml index ebc33da0..efbe1f2d 100644 --- a/charts/openshift-console-plugin/values.yaml +++ b/charts/openshift-console-plugin/values.yaml @@ -6,6 +6,9 @@ plugin: apiServerURL: "" ghApiUrl: "" funcCliVersion: "" + # saTokenExpiry is the deploy ServiceAccount token lifetime as a duration, + # e.g. 30d, 10h, or 7d12h. Empty uses the backend default (7 days). + saTokenExpiry: "" imagePullPolicy: IfNotPresent imagePullSecrets: [] replicas: 2 diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 8da54e3c..d65f5868 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -100,7 +100,7 @@ Go + `net/http` standard library. Key dependencies: | Dependency | Role | |---|---| | `k8s.io/client-go` | Kubernetes API client (SA, RBAC, TokenRequest, kubeconfig) | -| `google/go-github/v72` | GitHub API client | +| `google/go-github/v90` | GitHub API client | | `knative.dev/func` | Function scaffold generation | | `onsi/ginkgo` + `onsi/gomega` | Test framework | @@ -114,7 +114,7 @@ Go + `net/http` standard library. Key dependencies: | `handler` | HTTP handlers: input validation, orchestration, error mapping | | `scm` | SCM abstraction types (`Platform`, `Registry`, `Client`) and filesystem helpers | | `scm/github` | go-github implementation of `scm.Client` | -| `config` | Package-level wiring vars (`SCMRegistry`, constants) | +| `config` | Runtime configuration, package-level wiring vars (`SCMRegistry`), constants, and service account token expiry parsing | | `tlsreload` | Reloads the serving cert/key from disk on change (fsnotify plus a poll fallback), swapping an atomic `*tls.Certificate` via `GetCertificate` so rotated certs are served without a restart | ### Dependency Rules @@ -125,7 +125,7 @@ Go + `net/http` standard library. Key dependencies: - `cluster` is for provisioning (write RBAC/SA, request tokens); `functions` is for the function lifecycle (list and scaffold generation). Both talk to the cluster but answer different questions, so they stay separate rather than sharing one client interface - `scm` has no knowledge of cluster or functions - `functions` imports `scm` only for `scm.Platform` and `scm.FileEntry` types -- `config` is imported by `handler` and `main` only — it is the wiring layer +- `config` is imported by `handler`, `functions`, and `main` only; it owns runtime configuration and package-level wiring ### Key Decisions @@ -146,6 +146,9 @@ Everything that wraps `knative.dev/func` lives in this one package, so there is **External API URL resolved at Helm install time** The URL embedded in generated kubeconfigs (`externalAPIServerURL`) comes from the Infrastructure CR (`config.openshift.io/v1/Infrastructure/cluster`) via Helm `lookup` at install time, injected as `--external-api-server-url`. It is not fetched at runtime. This eliminates the need for a `ClusterRole` to query the Infrastructure CR from within the pod. +**Deployment credentials are short-lived and refreshed before file updates** +The backend requests service account tokens with a configurable lifetime, set by `--sa-token-expiry` and defaulting to seven days. When a function is created, its kubeconfig is stored as the `KUBECONFIG` SCM secret and the token expiration timestamp is stored as the `KUBECONFIG_EXPIRE_AT` SCM variable. Before pushing edited files, the handler refreshes both values when the token has 24 hours or less remaining. Missing or malformed expiration metadata also triggers a refresh. Refresh uses the caller's OCP bearer token and the namespace from the repository's `func.yaml`; no cluster credentials are retained by the backend. + **TLS serving certificate reloaded at runtime by an fsnotify + poll hybrid** The OCP service CA operator rotates the serving cert/key automatically. `tlsreload.Reloader` watches the mounted pair with fsnotify and atomically swaps the cached `*tls.Certificate` served via `tls.Config.GetCertificate`, so a rotation is picked up without restarting the pod. A poll ticker running every 30 seconds operates alongside the watcher as a safety net for events fsnotify can miss, in particular the atomic `..data` symlink swap Kubernetes uses for mounted secrets: when a watched file is removed or renamed the watch is re-added to the new file, and the poll guarantees the change is eventually observed regardless. Polling stays active during watcher setup failures and the 3-second watcher restart delays. Watcher events and poll ticks are processed on one goroutine, so `reload` and its content hash remain serialized and the swap needs only a plain atomic `Store` (no mutex). fsnotify was promoted from an existing indirect dependency; the heavier `k8s.io/apiserver/dynamiccertificates` and `controller-runtime/certwatcher` (which pulls in Prometheus) were avoided. Reloads are content-hashed to skip re-parsing when the pair is unchanged, and the last valid pair is retained when an update is incomplete or invalid. @@ -165,4 +168,4 @@ The OCP service CA operator rotates the serving cert/key automatically. `tlsrelo **Handlers are stateless** -`Handlers` holds only static config. Every request creates its own cluster client authenticated with the caller's OCP bearer token — there is no shared connection or session. +`Handlers` holds only static configuration, including the requested service account token lifetime. Cluster clients are request-scoped, authenticated with the caller's OCP bearer token, and created only when an operation needs cluster access. There is no shared connection, credential, or session.