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..95ca52ab --- /dev/null +++ b/backend/config/expiry.go @@ -0,0 +1,54 @@ +package config + +import ( + "errors" + "fmt" + "strconv" + "strings" + "time" +) + +const DefaultSATokenExpiry int64 = 7 * 24 * 60 * 60 // 7 days + +// 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/handler/create.go b/backend/handler/create.go index 28eee07a..56c11d2d 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" @@ -19,6 +20,7 @@ import ( const ( repoSecretKubeconfig = "KUBECONFIG" + repoSecretExpireAt = "KUBECONFIG_EXPIRE_AT" repoVarClusterAPIURL = "CLUSTER_API_URL" ) @@ -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, repoSecretExpireAt, 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..5782adff 100644 --- a/backend/handler/files.go +++ b/backend/handler/files.go @@ -7,13 +7,18 @@ 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])?$`) +const tokenRefreshWindow = 24 * time.Hour + func (h *Handlers) HandleGetFiles(w http.ResponseWriter, r *http.Request) { pat, ok := extractSCMToken(r) if !ok { @@ -59,6 +64,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 +109,128 @@ 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 !h.refreshDeploymentCredentialsIfNeeded(w, r, client, target) { + return + } + if !pushUpdatedFiles(w, r, client, target, req) { + return + } + + w.WriteHeader(http.StatusNoContent) +} + +func (h *Handlers) refreshDeploymentCredentialsIfNeeded(w http.ResponseWriter, r *http.Request, client scm.Client, target putFilesTarget) bool { + expiration, err := client.GetVariable(r.Context(), target.owner, target.repo, repoSecretExpireAt) + if err != nil { if errors.Is(err, scm.ErrUnauthorized) { writeError(w, http.StatusUnauthorized, "invalid SCM token") - return + return false + } + slog.Error("failed to read deployment credential expiration", "owner", target.owner, "repo", target.repo, "err", err) + writeError(w, http.StatusBadGateway, "failed to check deployment credentials") + return false + } + + if !tokenNeedsRefresh(expiration, time.Now()) { + return true + } + + ocpToken, ok := extractOCPToken(r) + if !ok { + writeError(w, http.StatusUnauthorized, "Authorization header is required") + return false + } + + funcYaml, err := client.GetFileContent(r.Context(), target.owner, target.repo, target.branch, "func.yaml") + if err != nil { + if errors.Is(err, scm.ErrUnauthorized) { + writeError(w, http.StatusUnauthorized, "invalid SCM token") + return false + } + slog.Error("failed to read func.yaml", "owner", target.owner, "repo", target.repo, "branch", target.branch, "err", err) + writeError(w, http.StatusBadGateway, "failed to read function configuration") + return false + } + _, 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) + writeError(w, http.StatusUnprocessableEntity, "invalid function configuration") + return false + } + if errs := k8svalidation.IsDNS1123Label(namespace); len(errs) > 0 { + slog.Error("invalid namespace in func.yaml", "owner", target.owner, "repo", target.repo, "branch", target.branch) + writeError(w, http.StatusUnprocessableEntity, "invalid namespace in function configuration") + return false + } + + clusterClient, err := newClusterClient(h.kubeHost, ocpToken, h.caCert) + if err != nil { + slog.Error("failed to connect to cluster", "namespace", namespace, "err", err) + writeError(w, http.StatusBadGateway, "failed to refresh deployment credentials") + return false + } + + 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) + writeError(w, http.StatusBadGateway, "failed to refresh deployment credentials") + return false + } + + 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) + writeError(w, http.StatusBadGateway, "failed to refresh deployment credentials") + return false + } + if err := client.StoreSecret(r.Context(), target.owner, target.repo, repoSecretKubeconfig, kubeconfig); err != nil { + if errors.Is(err, scm.ErrUnauthorized) { + writeError(w, http.StatusUnauthorized, "invalid SCM token") + return false + } + slog.Error("failed to update CI secret", "owner", target.owner, "repo", target.repo, "err", err) + writeError(w, http.StatusBadGateway, "failed to update deployment secret") + return false + } + if err := client.StoreVariable(r.Context(), target.owner, target.repo, repoSecretExpireAt, tokenStatus.ExpirationTimestamp.Time.UTC().Format(time.RFC3339)); err != nil { + if errors.Is(err, scm.ErrUnauthorized) { + writeError(w, http.StatusUnauthorized, "invalid SCM token") + return false + } + slog.Error("failed to update deployment credential expiration", "owner", target.owner, "repo", target.repo, "err", err) + writeError(w, http.StatusBadGateway, "failed to update deployment credentials") + return false + } + + return true +} + +func pushUpdatedFiles(w http.ResponseWriter, r *http.Request, client scm.Client, target putFilesTarget, req putFilesRequest) bool { + 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 false } - 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 + return false } - w.WriteHeader(http.StatusNoContent) + return true +} + +func tokenNeedsRefresh(expiration string, now time.Time) bool { + if expiration == "" { + return true + } + + expiresAt, err := time.Parse(time.RFC3339, expiration) + if err != nil { + expiresAt, err = time.Parse("2006-01-02 15:04:05.999999999 -0700 MST", expiration) + if err != nil { + return true + } + } + return expiresAt.Sub(now) <= tokenRefreshWindow } diff --git a/backend/handler/files_refresh_test.go b/backend/handler/files_refresh_test.go new file mode 100644 index 00000000..8f7ed8ce --- /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/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(repoSecretExpireAt)) + 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(repoSecretExpireAt)) + 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(24 * time.Hour).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(24*time.Hour + 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"), + Entry("legacy Kubernetes timestamp", now.Add(12*time.Hour).Format("2006-01-02 15:04:05 -0700 MST")), + ) +}) 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..041ec007 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 + saTokenExpiry int64 // requested SA token lifetime in seconds } -func New(caPath, kubeHost, externalAPIServerURL string) (*Handlers, error) { +func New(caPath, kubeHost, externalAPIServerURL string, saTokenExpiry int64) (*Handlers, error) { var caCert []byte if caPath != "" { var err error @@ -24,7 +25,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