From c11e29bc9208e6ab4f17dc140f2084c0a46676e8 Mon Sep 17 00:00:00 2001 From: Tiffany Pei Date: Tue, 17 Sep 2024 20:21:05 +0000 Subject: [PATCH] Support pre-sync annotations --- e2e/testcases/presync_test.go | 197 ++++++++++++++++++++++++++++++++++ pkg/metadata/annotations.go | 10 ++ pkg/parse/namespace.go | 36 +++++++ pkg/parse/opts.go | 1 + pkg/parse/root.go | 36 +++++++ pkg/parse/run.go | 10 ++ pkg/parse/run_test.go | 21 ++-- 7 files changed, 304 insertions(+), 7 deletions(-) create mode 100644 e2e/testcases/presync_test.go diff --git a/e2e/testcases/presync_test.go b/e2e/testcases/presync_test.go new file mode 100644 index 0000000000..b57c4a5fc4 --- /dev/null +++ b/e2e/testcases/presync_test.go @@ -0,0 +1,197 @@ +// Copyright 2024 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package e2e + +import ( + "fmt" + "testing" + + "kpt.dev/configsync/e2e/nomostest" + "kpt.dev/configsync/e2e/nomostest/ntopts" + "kpt.dev/configsync/e2e/nomostest/policy" + "kpt.dev/configsync/e2e/nomostest/registryproviders" + nomostesting "kpt.dev/configsync/e2e/nomostest/testing" + "kpt.dev/configsync/e2e/nomostest/testpredicates" + "kpt.dev/configsync/e2e/nomostest/testwatcher" + "kpt.dev/configsync/pkg/api/configsync" + "kpt.dev/configsync/pkg/api/configsync/v1beta1" + "kpt.dev/configsync/pkg/core" + "kpt.dev/configsync/pkg/core/k8sobjects" + "kpt.dev/configsync/pkg/kinds" + "kpt.dev/configsync/pkg/metadata" + "kpt.dev/configsync/pkg/reposync" + "kpt.dev/configsync/pkg/rootsync" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +func TestAddPreSyncAnnotationRepoSync(t *testing.T) { + repoSyncID := core.RepoSyncID(configsync.RepoSyncName, namespaceRepo) + nt := nomostest.New(t, nomostesting.SyncSource, + ntopts.RequireOCIProvider, + ntopts.SyncWithGitSource(repoSyncID), + ntopts.RepoSyncPermissions(policy.RBACAdmin(), policy.CoreAdmin())) + rootSyncGitRepo := nt.SyncSourceGitReadWriteRepository(nomostest.DefaultRootSyncID) + repoSyncKey := repoSyncID.ObjectKey + gitSource := nt.SyncSources[repoSyncID] + bookinfoRole := k8sobjects.RoleObject(core.Name("bookinfo-admin")) + image, err := nt.BuildAndPushOCIImage(repoSyncKey, registryproviders.ImageInputObjects(nt.Scheme, bookinfoRole)) + if err != nil { + nt.T.Fatal(err) + } + nt.T.Log("Create RepoSync with OCI image") + repoSyncOCI := nt.RepoSyncObjectOCI(repoSyncKey, image.OCIImageID().WithoutDigest(), "", image.Digest) + nt.Must(rootSyncGitRepo.Add(nomostest.StructuredNSPath(repoSyncID.Namespace, repoSyncID.Name), repoSyncOCI)) + nt.Must(rootSyncGitRepo.CommitAndPush("Set the RepoSync to sync from OCI")) + nt.Must(nt.WatchForAllSyncs()) + + err = nt.Watcher.WatchObject(kinds.RepoSyncV1Beta1(), repoSyncID.Name, repoSyncID.Namespace, + testwatcher.WatchPredicates( + checkRepoSyncPreSyncAnnotations(), + )) + if err != nil { + nt.T.Fatalf("Source annotation not updated for RepoSync %v", err) + } + nt.T.Log("Set the RepoSync to sync from Git") + nt.SyncSources[repoSyncID] = gitSource + repoSyncGit := nomostest.RepoSyncObjectV1Beta1FromNonRootRepo(nt, repoSyncID.ObjectKey) + nt.Must(rootSyncGitRepo.Add(nomostest.StructuredNSPath(repoSyncID.Namespace, repoSyncID.Name), repoSyncGit)) + nt.Must(rootSyncGitRepo.CommitAndPush("Set the RepoSync to sync from Git")) + nt.Must(nt.WatchForAllSyncs()) + + err = nt.Watcher.WatchObject(kinds.RepoSyncV1Beta1(), repoSyncID.Name, repoSyncID.Namespace, + testwatcher.WatchPredicates( + testpredicates.MissingAnnotation(metadata.SourceURLAnnotationKey), + )) + if err != nil { + nt.T.Fatalf("Source annotations still exist when RepoSync is syncing from Git %v", err) + } +} + +func TestAddPreSyncAnnotationRootSync(t *testing.T) { + rootSyncID := nomostest.DefaultRootSyncID + rootSyncKey := rootSyncID.ObjectKey + nt := nomostest.New(t, nomostesting.SyncSource, + ntopts.SyncWithGitSource(rootSyncID, ntopts.Unstructured), + ntopts.RequireOCIProvider, + ) + gitSource := nt.SyncSources[rootSyncID] + bookinfoRole := k8sobjects.RoleObject(core.Name("bookinfo-admin")) + image, err := nt.BuildAndPushOCIImage(rootSyncKey, registryproviders.ImageInputObjects(nt.Scheme, bookinfoRole)) + if err != nil { + nt.T.Fatal(err) + } + nt.T.Log("Create RootSync with OCI image") + rootSyncOCI := nt.RootSyncObjectOCI(rootSyncKey.Name, image.OCIImageID().WithoutDigest(), "", image.Digest) + nt.Must(nt.KubeClient.Apply(rootSyncOCI)) + nt.Must(nt.WatchForAllSyncs()) + + err = nt.Watcher.WatchObject(kinds.RootSyncV1Beta1(), rootSyncID.Name, rootSyncID.Namespace, + testwatcher.WatchPredicates( + checkRootSyncPreSyncAnnotations(), + )) + if err != nil { + nt.T.Fatalf("Source annotation not updated for RootSync %v", err) + } + + nt.T.Log("Set the RootSync to sync from Git") + nt.SyncSources[rootSyncID] = gitSource + rootSyncGit := nomostest.RootSyncObjectV1Beta1FromRootRepo(nt, rootSyncID.Name) + nt.Must(nt.KubeClient.Apply(rootSyncGit)) + nt.Must(nt.WatchForAllSyncs()) + + err = nt.Watcher.WatchObject(kinds.RootSyncV1Beta1(), rootSyncID.Name, rootSyncID.Namespace, + testwatcher.WatchPredicates( + testpredicates.MissingAnnotation(metadata.SourceURLAnnotationKey), + )) + if err != nil { + nt.T.Fatalf("Source annotations still exist when RootSync is syncing from Git %v", err) + } +} + +func checkRootSyncPreSyncAnnotations() testpredicates.Predicate { + return func(o client.Object) error { + if o == nil { + return testpredicates.ErrObjectNotFound + } + rs, ok := o.(*v1beta1.RootSync) + if !ok { + return testpredicates.WrongTypeErr(o, &v1beta1.RootSync{}) + } + syncingCondition := rootsync.GetCondition(rs.Status.Conditions, v1beta1.RootSyncSyncing) + syncingCommit := syncingCondition.Commit + syncingImage := rs.Spec.Oci.Image + commitAnnotation, ok := o.GetAnnotations()[metadata.SourceCommitAnnotationKey] + if !ok { + return fmt.Errorf("object %q does not have annotation %q", o.GetName(), metadata.SourceCommitAnnotationKey) + } + repoAnnotation, ok := o.GetAnnotations()[metadata.SourceURLAnnotationKey] + if !ok { + return fmt.Errorf("object %q does not have annotation %q", o.GetName(), metadata.SourceURLAnnotationKey) + } + if syncingCommit != commitAnnotation { + return fmt.Errorf("object %s has commit annotation %s, but is being synced with commit %s", + o.GetName(), + commitAnnotation, + syncingCommit, + ) + } + if syncingImage != repoAnnotation { + return fmt.Errorf("object %s has commit annotation %s, but is being synced with commit %s", + o.GetName(), + commitAnnotation, + syncingCommit, + ) + } + return nil + } +} + +func checkRepoSyncPreSyncAnnotations() testpredicates.Predicate { + return func(o client.Object) error { + if o == nil { + return testpredicates.ErrObjectNotFound + } + rs, ok := o.(*v1beta1.RepoSync) + if !ok { + return testpredicates.WrongTypeErr(o, &v1beta1.RepoSync{}) + } + syncingCondition := reposync.GetCondition(rs.Status.Conditions, v1beta1.RepoSyncSyncing) + syncingCommit := syncingCondition.Commit + syncingImage := rs.Spec.Oci.Image + commitAnnotation, ok := o.GetAnnotations()[metadata.SourceCommitAnnotationKey] + if !ok { + return fmt.Errorf("object %q does not have annotation %q", o.GetName(), metadata.SourceCommitAnnotationKey) + } + repoAnnotation, ok := o.GetAnnotations()[metadata.SourceURLAnnotationKey] + if !ok { + return fmt.Errorf("object %q does not have annotation %q", o.GetName(), metadata.SourceURLAnnotationKey) + } + if syncingCommit != commitAnnotation { + return fmt.Errorf("object %s has commit annotation %s, but is being synced with commit %s", + o.GetName(), + commitAnnotation, + syncingCommit, + ) + } + if syncingImage != repoAnnotation { + return fmt.Errorf("object %s has commit annotation %s, but is being synced with commit %s", + o.GetName(), + commitAnnotation, + syncingCommit, + ) + } + return nil + } +} diff --git a/pkg/metadata/annotations.go b/pkg/metadata/annotations.go index 37cc36d26f..3c8e04b924 100644 --- a/pkg/metadata/annotations.go +++ b/pkg/metadata/annotations.go @@ -144,6 +144,16 @@ const ( // reconciler-manager reads it. If set to true, the reconciler-manager will // create the reconciler with the Namespace controller in the reconciler container. DynamicNSSelectorEnabledAnnotationKey = configsync.ConfigSyncPrefix + "dynamic-ns-selector-enabled" + + // SourceURLAnnotationKey stores the source URL where the source commit is fetched from. + // It is only set for OCI and Helm source, and unset for the Git source. + // It can be used for OCI signature verification. + SourceURLAnnotationKey = configsync.ConfigSyncPrefix + "source-url" + + // SourceCommitAnnotationKey stores the Git source commit fetched by git-sync, + // the OCI image digest fetched by oci-sync, or the Helm chart version fetched by helm-sync. + // It can be used for OCI signature verification. + SourceCommitAnnotationKey = configsync.ConfigSyncPrefix + "source-commit" ) // Lifecycle annotations diff --git a/pkg/parse/namespace.go b/pkg/parse/namespace.go index 93a4ba6dc0..361af2a037 100644 --- a/pkg/parse/namespace.go +++ b/pkg/parse/namespace.go @@ -17,6 +17,7 @@ package parse import ( "context" "fmt" + "reflect" "strconv" "github.com/google/go-cmp/cmp" @@ -174,6 +175,41 @@ func (p *namespace) setSourceStatusWithRetries(ctx context.Context, newStatus *S return nil } +func (p *namespace) setSourceAnnotations(ctx context.Context, commit string) error { + rs := &v1beta1.RepoSync{} + if err := p.Client.Get(ctx, reposync.ObjectKey(p.Scope, p.SyncName), rs); err != nil { + return status.APIServerError(err, "failed to get RepoSync for parser") + } + existing := rs.DeepCopy() + + // Always update the source-commit annotation + currentSourceCommit := rs.GetAnnotations()[metadata.SourceCommitAnnotationKey] + if commit != currentSourceCommit { + core.SetAnnotation(rs, metadata.SourceCommitAnnotationKey, commit) + } + + // Update the source-url annotation based on the source type + var newSourceURL string + if p.Options.SourceType == configsync.OciSource || p.Options.SourceType == configsync.HelmSource { + newSourceURL = p.Options.SourceRepo + } + + currentImageURL := rs.GetAnnotations()[metadata.SourceURLAnnotationKey] + if newSourceURL != currentImageURL { + if newSourceURL == "" { + core.RemoveAnnotations(rs, metadata.SourceURLAnnotationKey) + } else { + core.SetAnnotation(rs, metadata.SourceURLAnnotationKey, newSourceURL) + } + } + + // Patch the RepoSync if any annotations were updated + if !reflect.DeepEqual(existing.GetAnnotations(), rs.GetAnnotations()) { + return p.Client.Patch(ctx, rs, client.MergeFrom(existing), client.FieldOwner(configsync.FieldManager)) + } + return nil +} + func (p *namespace) setRequiresRendering(ctx context.Context, renderingRequired bool) error { rs := &v1beta1.RepoSync{} if err := p.Client.Get(ctx, reposync.ObjectKey(p.Scope, p.SyncName), rs); err != nil { diff --git a/pkg/parse/opts.go b/pkg/parse/opts.go index 6c4d5ca1a2..e3656dbbbf 100644 --- a/pkg/parse/opts.go +++ b/pkg/parse/opts.go @@ -95,6 +95,7 @@ type Parser interface { K8sClient() client.Client // setRequiresRendering sets the requires-rendering annotation on the RSync setRequiresRendering(ctx context.Context, renderingRequired bool) error + setSourceAnnotations(ctx context.Context, commit string) error } func (o *Options) k8sClient() client.Client { diff --git a/pkg/parse/root.go b/pkg/parse/root.go index 138fb02770..c0f5b2a517 100644 --- a/pkg/parse/root.go +++ b/pkg/parse/root.go @@ -17,6 +17,7 @@ package parse import ( "context" "fmt" + "reflect" "strconv" "github.com/elliotchance/orderedmap/v2" @@ -261,6 +262,41 @@ func setSourceStatusFields(source *v1beta1.SourceStatus, newStatus *SourceStatus source.LastUpdate = newStatus.LastUpdate } +func (p *root) setSourceAnnotations(ctx context.Context, commit string) error { + rs := &v1beta1.RootSync{} + if err := p.Client.Get(ctx, rootsync.ObjectKey(p.SyncName), rs); err != nil { + return status.APIServerError(err, "failed to get RootSync for parser") + } + existing := rs.DeepCopy() + + // Always update the source-commit annotation + currentSourceCommit := rs.GetAnnotations()[metadata.SourceCommitAnnotationKey] + if commit != currentSourceCommit { + core.SetAnnotation(rs, metadata.SourceCommitAnnotationKey, commit) + } + + // Update the source-url annotation based on the source type + var newSourceURL string + if p.Options.SourceType == configsync.OciSource || p.Options.SourceType == configsync.HelmSource { + newSourceURL = p.Options.SourceRepo + } + + currentImageURL := rs.GetAnnotations()[metadata.SourceURLAnnotationKey] + if newSourceURL != currentImageURL { + if newSourceURL == "" { + core.RemoveAnnotations(rs, metadata.SourceURLAnnotationKey) + } else { + core.SetAnnotation(rs, metadata.SourceURLAnnotationKey, newSourceURL) + } + } + + // Patch the RepoSync if any annotations were updated + if !reflect.DeepEqual(existing.GetAnnotations(), rs.GetAnnotations()) { + return p.Client.Patch(ctx, rs, client.MergeFrom(existing), client.FieldOwner(configsync.FieldManager)) + } + return nil +} + func (p *root) setRequiresRendering(ctx context.Context, renderingRequired bool) error { rs := &v1beta1.RootSync{} if err := p.Client.Get(ctx, rootsync.ObjectKey(p.SyncName), rs); err != nil { diff --git a/pkg/parse/run.go b/pkg/parse/run.go index 606df69955..66cd3adf57 100644 --- a/pkg/parse/run.go +++ b/pkg/parse/run.go @@ -95,6 +95,16 @@ func DefaultRunFunc(ctx context.Context, p Parser, trigger string, state *reconc // pull the source commit and directory with retries within 5 minutes. gs.Commit, syncDir, gs.Errs = hydrate.SourceCommitAndDirWithRetry(util.SourceRetryBackoff, opts.SourceType, opts.SourceDir, opts.SyncDir, opts.ReconcilerName) + // Add pre-sync annotations to the object. + // If updating the object fails, it's likely due to a signature verification error + // from the webhook. In this case, add the error as a source error. + if gs.Errs == nil { + err := p.setSourceAnnotations(ctx, gs.Commit) + if err != nil { + gs.Errs = status.Append(gs.Errs, status.SourceError.Wrap(err).Build()) + } + } + // Generate source spec from Reconciler config gs.Spec = SourceSpecFromFileSource(opts.FileSource, opts.SourceType, gs.Commit) diff --git a/pkg/parse/run_test.go b/pkg/parse/run_test.go index ac6e8e9e76..3be87b81da 100644 --- a/pkg/parse/run_test.go +++ b/pkg/parse/run_test.go @@ -270,7 +270,8 @@ func TestRun(t *testing.T) { expectedRootSync: func() *v1beta1.RootSync { rs := rootSyncOutput.DeepCopy() // Create + Update (source status) + Update (rendering status) + Update (sync status) - rs.ObjectMeta.ResourceVersion = "4" + rs.ObjectMeta.ResourceVersion = "5" + core.SetAnnotation(rs, metadata.SourceCommitAnnotationKey, "abcd123") rs.Status.Status.LastSyncedCommit = sourceCommit rs.Status.Status.Source = v1beta1.SourceStatus{ Git: &v1beta1.GitStatus{ @@ -363,7 +364,8 @@ func TestRun(t *testing.T) { expectedRootSync: func() *v1beta1.RootSync { rs := rootSyncOutput.DeepCopy() // Create + Update (source status) + Update (rendering status) - rs.ObjectMeta.ResourceVersion = "3" + rs.ObjectMeta.ResourceVersion = "4" + core.SetAnnotation(rs, metadata.SourceCommitAnnotationKey, "abcd123") rs.Status.Status.Source = v1beta1.SourceStatus{ Git: &v1beta1.GitStatus{ Repo: fileSource.SourceRepo, @@ -410,7 +412,8 @@ func TestRun(t *testing.T) { expectedRootSync: func() *v1beta1.RootSync { rs := rootSyncOutput.DeepCopy() // Create + Update (source status) + Update (rendering status) - rs.ObjectMeta.ResourceVersion = "3" + rs.ObjectMeta.ResourceVersion = "4" + core.SetAnnotation(rs, metadata.SourceCommitAnnotationKey, "abcd123") rs.Status.Status.Source = v1beta1.SourceStatus{ Git: &v1beta1.GitStatus{ Repo: fileSource.SourceRepo, @@ -462,7 +465,8 @@ func TestRun(t *testing.T) { expectedRootSync: func() *v1beta1.RootSync { rs := rootSyncOutput.DeepCopy() // Create + Update (source status) + Update (rendering status) + Update (sync status) - rs.ObjectMeta.ResourceVersion = "4" + rs.ObjectMeta.ResourceVersion = "5" + core.SetAnnotation(rs, metadata.SourceCommitAnnotationKey, "abcd123") rs.Status.Status.LastSyncedCommit = sourceCommit rs.Status.Status.Source = v1beta1.SourceStatus{ Git: &v1beta1.GitStatus{ @@ -516,7 +520,8 @@ func TestRun(t *testing.T) { expectedRootSync: func() *v1beta1.RootSync { rs := rootSyncOutput.DeepCopy() // Create + Update (source status) + Update (rendering status) + Update (sync status) - rs.ObjectMeta.ResourceVersion = "4" + rs.ObjectMeta.ResourceVersion = "5" + core.SetAnnotation(rs, metadata.SourceCommitAnnotationKey, "abcd123") rs.Status.Status.LastSyncedCommit = sourceCommit rs.Status.Status.Source = v1beta1.SourceStatus{ Git: &v1beta1.GitStatus{ @@ -572,9 +577,10 @@ func TestRun(t *testing.T) { expectedRootSync: func() *v1beta1.RootSync { rs := rootSyncOutput.DeepCopy() // Create + Update (source status) + Update (rendering status) - rs.ObjectMeta.ResourceVersion = "4" // TODO: Why 4 and not just 3? + rs.ObjectMeta.ResourceVersion = "5" // TODO: Why 4 and not just 3? // Tell reconciler-manager to disable the hydration-controller container core.SetAnnotation(rs, metadata.RequiresRenderingAnnotationKey, "false") + core.SetAnnotation(rs, metadata.SourceCommitAnnotationKey, "abcd123") rs.Status.Status.Source = v1beta1.SourceStatus{ Git: &v1beta1.GitStatus{ Repo: fileSource.SourceRepo, @@ -625,9 +631,10 @@ func TestRun(t *testing.T) { expectedRootSync: func() *v1beta1.RootSync { rs := rootSyncOutput.DeepCopy() // Create + Update (source status) + Update (rendering status) - rs.ObjectMeta.ResourceVersion = "4" // TODO: Why 4 and not just 3? + rs.ObjectMeta.ResourceVersion = "5" // TODO: Why 4 and not just 3? // Tell reconciler-manager to enable the hydration-controller container core.SetAnnotation(rs, metadata.RequiresRenderingAnnotationKey, "true") + core.SetAnnotation(rs, metadata.SourceCommitAnnotationKey, "abcd123") rs.Status.Status.Source = v1beta1.SourceStatus{ Git: &v1beta1.GitStatus{ Repo: fileSource.SourceRepo,