e2e: refactor sync expectations (#1410)

- Add Branch & Revision to GitSyncSource to configure root-sync
  and other test setup RSyncs.
- Add SetExpectedSyncSource & UpdateExpectedSyncSource
- Add ExpectedCommit, ExpectedChartVersion, & ExpectedImageDigest
- Add ExpectedDirectory for OCI and Git
- Remove WithRootSha1Func & WithRepoSha1Func.
  Use one of the Expected- fields in the SyncSource instead.
- Update test setup to provision RSyncs without overwriting any
  existing SyncSource expectations.

e2e/nomostest/config_sync_sources.go

@@ -15,125 +15,82 @@
 package nomostest
 
 import (
-	"kpt.dev/configsync/e2e/nomostest/gitproviders"
-	"kpt.dev/configsync/e2e/nomostest/registryproviders"
+	"reflect"
+
 	"kpt.dev/configsync/e2e/nomostest/syncsource"
-	"kpt.dev/configsync/pkg/api/configsync"
 	"kpt.dev/configsync/pkg/core"
 )
 
-// SetExpectedSyncPath updates the SyncSource for the specified RootSync or
-// RepoSync with the provided Git dir, OCI dir.
-func SetExpectedSyncPath(nt *NT, syncID core.ID, syncPath string) {
-	source, exists := nt.SyncSources[syncID]
+// SetExpectedSyncSource creates, updates, or replaces the SyncSource for the
+// specified RootSync or RepoSync.
+func SetExpectedSyncSource(nt *NT, syncID core.ID, source syncsource.SyncSource) {
+	oldSource, exists := nt.SyncSources[syncID]
 	if !exists {
-		nt.T.Fatalf("Failed to update expectation for %s %s: nt.SyncSources not registered", syncID.Kind, syncID.ObjectKey)
-	}
-	switch tSource := source.(type) {
-	case *syncsource.GitSyncSource:
-		tSource.Directory = syncPath
-	case *syncsource.OCISyncSource:
-		tSource.Directory = syncPath
-	case *syncsource.HelmSyncSource:
-		nt.T.Fatalf("Failed to update expectation for %s %s: Use SetExpectedHelmChart instead", syncID.Kind, syncID.ObjectKey)
-	default:
-		nt.T.Fatalf("Invalid %s source %T: %s", syncID.Kind, source, syncID.Name)
+		nt.T.Logf("Creating expectation for %s %s to sync with %T (%s)",
+			syncID.Kind, syncID.ObjectKey, source, source)
+	} else if reflect.TypeOf(oldSource) == reflect.TypeOf(source) {
+		nt.T.Logf("Updating expectation for %s %s to sync with %T (%s)",
+			syncID.Kind, syncID.ObjectKey, source, source)
+	} else {
+		nt.T.Logf("Replacing expectation for %s %s to sync with %T (%s), instead of %T",
+			syncID.Kind, syncID.ObjectKey, source, source, oldSource)
 	}
+	nt.SyncSources[syncID] = source
 }
 
-// SetExpectedGitSource creates, updates, or replaces the SyncSource for the
-// specified RootSync or RepoSync with the provided Git repo.
-func SetExpectedGitSource(nt *NT, syncID core.ID, repo *gitproviders.ReadWriteRepository, syncPath string, sourceFormat configsync.SourceFormat) {
+// UpdateExpectedSyncSource executes the provided function on an existing
+// SyncSource in the SyncSources, allowing updating multiple fields.
+func UpdateExpectedSyncSource[T syncsource.SyncSource](nt *NT, syncID core.ID, mutateFn func(T)) {
 	source, exists := nt.SyncSources[syncID]
 	if !exists {
-		nt.T.Logf("Creating expectation for %s %s to sync with Git repo (repository: %q, directory: %q, sourceFormat: %q)",
-			syncID.Kind, syncID.ObjectKey, repo.Name, syncPath, sourceFormat)
-		nt.SyncSources[syncID] = &syncsource.GitSyncSource{
-			Repository:   repo,
-			SourceFormat: sourceFormat,
-			Directory:    syncPath,
-		}
-		return
+		nt.T.Fatalf("Failed to update expectation for %s %s: nt.SyncSources not registered", syncID.Kind, syncID.ObjectKey)
 	}
 	switch tSource := source.(type) {
-	case *syncsource.GitSyncSource:
-		nt.T.Logf("Updating expectation for %s %s to sync with Git repo (repository: %q, directory: %q, sourceFormat: %q)",
-			syncID.Kind, syncID.ObjectKey, repo.Name, syncPath, sourceFormat)
-		tSource.Repository = repo
-		tSource.SourceFormat = sourceFormat
-		tSource.Directory = syncPath
-	case *syncsource.OCISyncSource, *syncsource.HelmSyncSource:
-		nt.T.Logf("Replacing expectation for %s %s to sync with Git repo (repository: %q, directory: %q, sourceFormat: %q), instead of %T",
-			syncID.Kind, syncID.ObjectKey, repo.Name, syncPath, sourceFormat, source)
-		nt.SyncSources[syncID] = &syncsource.GitSyncSource{
-			Repository:   repo,
-			SourceFormat: sourceFormat,
-			Directory:    syncPath,
-		}
+	case T:
+		mutateFn(tSource)
+		nt.T.Logf("Updating expectation for %s %s to sync with %T (%s)",
+			syncID.Kind, syncID.ObjectKey, tSource, tSource)
 	default:
 		nt.T.Fatalf("Invalid %s source %T: %s", syncID.Kind, source, syncID.Name)
 	}
 }
 
-// SetExpectedOCISource creates, updates, or replaces the SyncSource for the
-// specified RootSync or RepoSync with the provided OCI image.
-// ImageID is used for pulling. ImageDigest is used for validating the result.
-func SetExpectedOCISource(nt *NT, syncID core.ID, imageID registryproviders.OCIImageID, imageDigest, syncPath string) {
+// SetExpectedSyncPath updates the SyncSource for the specified RootSync or
+// RepoSync with the provided Git dir, OCI dir.
+// Updates both the Directory & ExpectedDirectory.
+func SetExpectedSyncPath(nt *NT, syncID core.ID, syncPath string) {
 	source, exists := nt.SyncSources[syncID]
 	if !exists {
-		nt.T.Logf("Creating expectation for %s %s to sync with OCI image (image: %q, directory: %q)",
-			syncID.Kind, syncID.ObjectKey, imageID, syncPath)
-		nt.SyncSources[syncID] = &syncsource.OCISyncSource{
-			ImageID:   imageID,
-			Digest:    imageDigest,
-			Directory: syncPath,
-		}
-		return
+		nt.T.Fatalf("Failed to update expectation for %s %s: nt.SyncSources not registered", syncID.Kind, syncID.ObjectKey)
 	}
 	switch tSource := source.(type) {
+	case *syncsource.GitSyncSource:
+		tSource.Directory = syncPath
+		tSource.ExpectedDirectory = ""
 	case *syncsource.OCISyncSource:
-		nt.T.Logf("Updating expectation for %s %s to sync with OCI image (image: %q, directory: %q)",
-			syncID.Kind, syncID.ObjectKey, imageID, syncPath)
-		tSource.ImageID = imageID
-		tSource.Digest = imageDigest
 		tSource.Directory = syncPath
-	case *syncsource.GitSyncSource, *syncsource.HelmSyncSource:
-		nt.T.Logf("Replacing expectation for %s %s to sync with OCI image (image: %q, directory: %q), instead of %T",
-			syncID.Kind, syncID.ObjectKey, imageID, syncPath, source)
-		nt.SyncSources[syncID] = &syncsource.OCISyncSource{
-			ImageID:   imageID,
-			Digest:    imageDigest,
-			Directory: syncPath,
-		}
+		tSource.ExpectedDirectory = ""
+	case *syncsource.HelmSyncSource:
+		nt.T.Fatalf("Failed to update expectation for %s %s: Use SetExpectedSyncSource instead", syncID.Kind, syncID.ObjectKey)
 	default:
 		nt.T.Fatalf("Invalid %s source %T: %s", syncID.Kind, source, syncID.Name)
 	}
 }
 
-// SetExpectedHelmSource creates, updates, or replaces the SyncSource for the
-// specified RootSync or RepoSync with the provided Helm chart ID.
-func SetExpectedHelmSource(nt *NT, syncID core.ID, chartID registryproviders.HelmChartID) {
-	source, exists := nt.SyncSources[syncID]
-	if !exists {
-		nt.T.Logf("Creating expectation for %s %s to sync with helm chart (id: %q)",
-			syncID.Kind, syncID.ObjectKey, chartID)
-		nt.SyncSources[syncID] = &syncsource.HelmSyncSource{
-			ChartID: chartID,
-		}
-		return
-	}
-	switch tSource := source.(type) {
-	case *syncsource.HelmSyncSource:
-		nt.T.Logf("Updating expectation for %s %s to sync with helm chart (id: %q)",
-			syncID.Kind, syncID.ObjectKey, chartID)
-		tSource.ChartID = chartID
-	case *syncsource.GitSyncSource, *syncsource.OCISyncSource:
-		nt.T.Logf("Replacing expectation for %s %s to sync with helm chart (id: %q), instead of %T",
-			syncID.Kind, syncID.ObjectKey, chartID, source)
-		nt.SyncSources[syncID] = &syncsource.HelmSyncSource{
-			ChartID: chartID,
-		}
-	default:
-		nt.T.Fatalf("Invalid %s source %T: %s", syncID.Kind, source, syncID.Name)
-	}
+// SetExpectedGitCommit updates the SyncSource for the specified RootSync or
+// RepoSync with the provided Git commit, without changing the git reference of
+// the source being pulled.
+func SetExpectedGitCommit(nt *NT, syncID core.ID, expectedCommit string) {
+	UpdateExpectedSyncSource(nt, syncID, func(source *syncsource.GitSyncSource) {
+		source.ExpectedCommit = expectedCommit
+	})
+}
+
+// SetExpectedOCIImageDigest updates the SyncSource for the specified RootSync
+// or RepoSync with the provided OCI image digest, without changing the ID of
+// the image being pulled.
+func SetExpectedOCIImageDigest(nt *NT, syncID core.ID, expectedImageDigest string) {
+	UpdateExpectedSyncSource(nt, syncID, func(source *syncsource.OCISyncSource) {
+		source.ExpectedImageDigest = expectedImageDigest
+	})
 }

e2e/nomostest/new.go

@@ -427,14 +427,6 @@ func setupTestCase(nt *NT, opts *ntopts.New) {
 	for id, source := range opts.SyncSources {
 		switch tSource := source.(type) {
 		case *syncsource.GitSyncSource:
-			if tSource.Repository == nil {
-				// Initialize the Repository
-				tSource.Repository = initRepository(nt, id.Kind, id.ObjectKey, tSource.SourceFormat)
-			}
-			if tSource.Directory == "" {
-				// Set the default Directory
-				tSource.Directory = gitproviders.DefaultSyncDir
-			}
 			if tSource.SourceFormat == "" {
 				// Set the default SourceFormat
 				switch id.Kind {
@@ -446,6 +438,19 @@ func setupTestCase(nt *NT, opts *ntopts.New) {
 					tSource.SourceFormat = configsync.SourceFormatUnstructured
 				}
 			}
+			if tSource.Repository == nil {
+				// Initialize the Repository
+				tSource.Repository = initRepository(nt, id.Kind, id.ObjectKey, tSource.SourceFormat)
+			}
+			if tSource.Branch == "" {
+				// Set the default Branch
+				tSource.Branch = gitproviders.MainBranch
+			}
+			if tSource.Directory == "" {
+				// Set the default Directory
+				tSource.Directory = gitproviders.DefaultSyncDir
+				tSource.ExpectedDirectory = "" // Uses Directory when empty
+			}
 			nt.SyncSources[id] = tSource
 		// case *syncsource.HelmSyncSource:
 		// case *syncsource.OCISyncSource:
@@ -504,11 +509,21 @@ func setupTestCase(nt *NT, opts *ntopts.New) {
 	nt.RenewClient()
 
 	// Initialize the base RootSync using SSA for field management
-	rs := RootSyncObjectV1Beta1FromRootRepo(nt, configsync.RootSyncName)
-	if err := nt.KubeClient.Apply(rs); err != nil {
-		nt.T.Fatal(err)
+	rootSyncID := DefaultRootSyncID
+	var rootSyncObj client.Object
+	switch tSource := nt.SyncSources[rootSyncID].(type) {
+	case *syncsource.GitSyncSource:
+		rootSyncObj = nt.RootSyncObjectGitSyncSource(rootSyncID.Name, tSource)
+		// TODO: setup OCI & Helm RootSyncs
+		// case *syncsource.HelmSyncSource:
+		// 	rootSyncObj = nt.RootSyncObjectHelmSyncSource(rootSyncID.Name, tSource)
+		// case *syncsource.OCISyncSource:
+		// 	rootSyncObj = nt.RootSyncObjectOCISyncSource(rootSyncID.Name, tSource)
+	default:
+		nt.T.Fatalf("Invalid RootSync source %T: %s", tSource, rootSyncID.Name)
 	}
-	// Wait for Config Sync to be ready and the base RootSync to be synced.
+	nt.Must(nt.KubeClient.Apply(rootSyncObj))
+
 	if err := WaitForConfigSyncReady(nt); err != nil {
 		nt.T.Fatalf("waiting for ConfigSync Deployments to become available: %v", err)
 	}

e2e/nomostest/nt.go

@@ -390,18 +390,24 @@ func (nt *NT) NumRepoSyncNamespaces() int {
 	return len(rsNamespaces)
 }
 
-// SyncSourceGitReadWriteRepository returns the git Repository for the specified RSync,
-// if it exists in NT.SyncSources.
+// SyncSourceGitReadWriteRepository returns the git ReadWriteRepository for the
+// specified RSync, if it exists in NT.SyncSources.
 func (nt *NT) SyncSourceGitReadWriteRepository(id core.ID) *gitproviders.ReadWriteRepository {
 	source, found := nt.SyncSources[id]
 	if !found {
 		nt.T.Fatalf("Missing %s: %s", id.Kind, id.ObjectKey)
 	}
 	gitSource, ok := source.(*syncsource.GitSyncSource)
 	if !ok {
-		nt.T.Fatalf("Expected *GitSyncSource for %s %s but found %T: %s", id.Kind, id.ObjectKey, source)
+		nt.T.Fatalf("Expected *GitSyncSource for %s %s, but found %T",
+			id.Kind, id.ObjectKey, source)
 	}
-	return gitSource.Repository
+	gitRepo, ok := gitSource.Repository.(*gitproviders.ReadWriteRepository)
+	if !ok {
+		nt.T.Fatalf("Expected GitSyncSource.Repository for %s %s to be a ReadWriteRepository, but found %T",
+			id.Kind, id.ObjectKey, gitSource.Repository)
+	}
+	return gitRepo
 }
 
 // DefaultRootSha1Fn is the default function to retrieve the commit hash of the root repo.
@@ -715,8 +721,12 @@ func (nt *NT) portForwardGitServer() {
 		allGitRepoMap := make(map[types.NamespacedName]*gitproviders.ReadWriteRepository)
 		for id, source := range nt.SyncSources {
 			if gitSource, ok := source.(*syncsource.GitSyncSource); ok {
-				allGitRepos = append(allGitRepos, id.ObjectKey)
-				allGitRepoMap[id.ObjectKey] = gitSource.Repository
+				if gitRepo, ok := gitSource.Repository.(*gitproviders.ReadWriteRepository); ok {
+					allGitRepos = append(allGitRepos, id.ObjectKey)
+					allGitRepoMap[id.ObjectKey] = gitRepo
+				}
+				// Ignore ReadOnlyRepository - doesn't need a proxy, because it
+				// doesn't run on our test cluster.
 			}
 		}
 		// re-init all repos
@@ -945,7 +955,7 @@ func RemoteRootRepoSha1Fn(nt *NT, nn types.NamespacedName) (string, error) {
 	if err := nt.KubeClient.Get(nn.Name, nn.Namespace, rs); err != nil {
 		return "", err
 	}
-	commit, err := gitCommitFromSpec(nt, rs.Spec.Git)
+	commit, err := GitCommitFromSpec(nt, rs.Spec.Git)
 	if err != nil {
 		return "", fmt.Errorf("failed to lookup git commit for RootSync: %w", err)
 	}
@@ -958,27 +968,23 @@ func RemoteNsRepoSha1Fn(nt *NT, nn types.NamespacedName) (string, error) {
 	if err := nt.KubeClient.Get(nn.Name, nn.Namespace, rs); err != nil {
 		return "", err
 	}
-	commit, err := gitCommitFromSpec(nt, rs.Spec.Git)
+	commit, err := GitCommitFromSpec(nt, rs.Spec.Git)
 	if err != nil {
 		return "", fmt.Errorf("failed to lookup git commit for RepoSync: %w", err)
 	}
 	return commit, nil
 }
 
-// gitCommitFromSpec returns the latest commit from a Git spec.
+// GitCommitFromSpec returns the latest commit from a Git spec.
 // Uses git ls-remote to avoid needing to clone the repo.
 // Warning: may not work if authentication is required.
-func gitCommitFromSpec(nt *NT, gitSpec *v1beta1.Git) (string, error) {
+func GitCommitFromSpec(nt *NT, gitSpec *v1beta1.Git) (string, error) {
 	if gitSpec == nil {
 		return "", errors.New("spec.git is nil")
 	}
 	if gitSpec.Repo == "" {
 		return "", errors.New("spec.git.repo is empty")
 	}
-	// revision specified
-	if gitSpec.Revision != "" && gitSpec.Revision != "HEAD" {
-		return gitSpec.Revision, nil
-	}
 	var pattern string
 	if gitSpec.Branch != "" {
 		// HEAD of specified branch
@@ -987,6 +993,10 @@ func gitCommitFromSpec(nt *NT, gitSpec *v1beta1.Git) (string, error) {
 		// HEAD of default branch
 		pattern = "HEAD"
 	}
+	// revision specified
+	if gitSpec.Revision != "" {
+		pattern = gitSpec.Revision
+	}
 	var args []string
 	if strings.Contains(gitSpec.Repo, testing.CSRHost) {
 		cloneDir, err := cloneCloudSourceRepo(nt, gitSpec.Repo)