From c919ff540fb74ec20f47359e81cc270990ba1053 Mon Sep 17 00:00:00 2001 From: Maisem Ali Date: Tue, 29 Aug 2023 12:43:22 -0700 Subject: [PATCH] cmd/k8s-operator,ipn/store/kubestore: patch secrets instead of updating We would call Update on the secret, but that was racey and would occasionaly fail. Instead use patch whenever we can. Fixes errors like ``` boot: 2023/08/29 01:03:53 failed to set serve config: sending serve config: updating config: writing ServeConfig to StateStore: Operation cannot be fulfilled on secrets "ts-webdav-kfrzv-0": the object has been modified; please apply your changes to the latest version and try again {"level":"error","ts":"2023-08-29T01:03:48Z","msg":"Reconciler error","controller":"ingress","controllerGroup":"networking.k8s.io","controllerKind":"Ingress","Ingress":{"name":"webdav","namespace":"default"},"namespace":"default","name":"webdav","reconcileID":"96f5cfed-7782-4834-9b75-b0950fd563ed","error":"failed to provision: failed to create or get API key secret: Operation cannot be fulfilled on secrets \"ts-webdav-kfrzv-0\": the object has been modified; please apply your changes to the latest version and try again","stacktrace":"sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler\n\tsigs.k8s.io/controller-runtime@v0.15.0/pkg/internal/controller/controller.go:324\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem\n\tsigs.k8s.io/controller-runtime@v0.15.0/pkg/internal/controller/controller.go:265\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2.2\n\tsigs.k8s.io/controller-runtime@v0.15.0/pkg/internal/controller/controller.go:226"} ``` Updates #502 Updates #7895 Signed-off-by: Maisem Ali --- cmd/k8s-operator/sts.go | 10 +++++----- ipn/store/kubestore/store_kube.go | 19 +++++++++++++++++++ kube/client.go | 7 ++++--- 3 files changed, 28 insertions(+), 8 deletions(-) diff --git a/cmd/k8s-operator/sts.go b/cmd/k8s-operator/sts.go index 131443e9b..9994f01ca 100644 --- a/cmd/k8s-operator/sts.go +++ b/cmd/k8s-operator/sts.go @@ -175,15 +175,15 @@ func (a *tailscaleSTSReconciler) createOrGetSecret(ctx context.Context, logger * Labels: stsC.ChildResourceLabels, }, } - alreadyExists := false + var orig *corev1.Secret // unmodified copy of secret if err := a.Get(ctx, client.ObjectKeyFromObject(secret), secret); err == nil { logger.Debugf("secret %s/%s already exists", secret.GetNamespace(), secret.GetName()) - alreadyExists = true + orig = secret.DeepCopy() } else if !apierrors.IsNotFound(err) { return "", err } - if !alreadyExists { + if orig == nil { // Secret doesn't exist yet, create one. Initially it contains // only the Tailscale authkey, but once Tailscale starts it'll // also store the daemon state. @@ -218,8 +218,8 @@ func (a *tailscaleSTSReconciler) createOrGetSecret(ctx context.Context, logger * } mak.Set(&secret.StringData, "serve-config", string(j)) } - if alreadyExists { - if err := a.Update(ctx, secret); err != nil { + if orig != nil { + if err := a.Patch(ctx, secret, client.MergeFrom(orig)); err != nil { return "", err } } else { diff --git a/ipn/store/kubestore/store_kube.go b/ipn/store/kubestore/store_kube.go index 65c73c1bd..3eb29898e 100644 --- a/ipn/store/kubestore/store_kube.go +++ b/ipn/store/kubestore/store_kube.go @@ -19,6 +19,7 @@ import ( // Store is an ipn.StateStore that uses a Kubernetes Secret for persistence. type Store struct { client *kube.Client + canPatch bool secretName string } @@ -28,8 +29,13 @@ func New(_ logger.Logf, secretName string) (*Store, error) { if err != nil { return nil, err } + canPatch, err := c.CheckSecretPermissions(context.Background(), secretName) + if err != nil { + return nil, err + } return &Store{ client: c, + canPatch: canPatch, secretName: secretName, }, nil } @@ -93,6 +99,19 @@ func (s *Store) WriteState(id ipn.StateKey, bs []byte) error { } return err } + if s.canPatch { + m := []kube.JSONPatch{ + { + Op: "add", + Path: "/data/" + sanitizeKey(id), + Value: bs, + }, + } + if err := s.client.JSONPatchSecret(ctx, s.secretName, m); err != nil { + return err + } + return nil + } secret.Data[sanitizeKey(id)] = bs if err := s.client.UpdateSecret(ctx, secret); err != nil { return err diff --git a/kube/client.go b/kube/client.go index ef62b74ce..f4befd1c8 100644 --- a/kube/client.go +++ b/kube/client.go @@ -233,15 +233,16 @@ func (c *Client) UpdateSecret(ctx context.Context, s *Secret) error { // // https://tools.ietf.org/html/rfc6902 type JSONPatch struct { - Op string `json:"op"` - Path string `json:"path"` + Op string `json:"op"` + Path string `json:"path"` + Value any `json:"value,omitempty"` } // JSONPatchSecret updates a secret in the Kubernetes API using a JSON patch. // It currently (2023-03-02) only supports the "remove" operation. func (c *Client) JSONPatchSecret(ctx context.Context, name string, patch []JSONPatch) error { for _, p := range patch { - if p.Op != "remove" { + if p.Op != "remove" && p.Op != "add" { panic(fmt.Errorf("unsupported JSON patch operation: %q", p.Op)) } }