diff --git a/cmd/promote/git/app_interface.go b/cmd/promote/git/app_interface.go index 69a40bf6e..773d1f79d 100644 --- a/cmd/promote/git/app_interface.go +++ b/cmd/promote/git/app_interface.go @@ -10,6 +10,7 @@ import ( "strings" "github.com/openshift/osdctl/cmd/promote/iexec" + "github.com/openshift/osdctl/cmd/promote/pathutil" kyaml "sigs.k8s.io/kustomize/kyaml/yaml" "gopkg.in/yaml.v3" @@ -330,7 +331,12 @@ func (a *AppInterface) CommitSaasAndAppYmlFile(saasFile, serviceName, commitMess } componentName := strings.TrimPrefix(serviceName, "saas-") - appYmlPath := filepath.Join(a.GitDirectory, "data", "services", componentName, "app.yml") + + appYmlPath, err := pathutil.DeriveAppYmlPath(a.GitDirectory, saasFile, componentName) + if err != nil { + return fmt.Errorf("failed to derive app.yml path: %v", err) + } + if err := a.GitExecutor.Run(a.GitDirectory, "git", "add", appYmlPath); err != nil { return fmt.Errorf("failed to add file %s: %v", appYmlPath, err) } diff --git a/cmd/promote/git/app_interface_test.go b/cmd/promote/git/app_interface_test.go index 08e4291f5..9113f66cb 100644 --- a/cmd/promote/git/app_interface_test.go +++ b/cmd/promote/git/app_interface_test.go @@ -631,6 +631,19 @@ func TestCommitSaasAndAppYmlFile(t *testing.T) { commitMsg: "test hotfix commit", wantErr: false, }, + { + name: "commits_osd_operator_saas_and_app_yml_successfully", + setupMock: func(m *MockExec, dir, saasFile, serviceName, commitMsg string) { + //osd-operators have the app.yml under osd-operators//app.yml + appYmlPath := filepath.Join(dir, "data", "services", "osd-operators", "managed-cluster-config", "app.yml") + m.On("Run", dir, "git", []string{"add", saasFile}).Return(nil) + m.On("Run", dir, "git", []string{"add", appYmlPath}).Return(nil) + m.On("Run", dir, "git", []string{"commit", "-m", commitMsg}).Return(nil) + }, + serviceName: "saas-managed-cluster-config", + commitMsg: "test hotfix commit for OSD operator", + wantErr: false, + }, { name: "fails_when_saas_file_add_fails", setupMock: func(m *MockExec, dir, saasFile, serviceName, commitMsg string) { @@ -671,7 +684,15 @@ func TestCommitSaasAndAppYmlFile(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { tmpDir := t.TempDir() - saasFile := filepath.Join(tmpDir, "saas.yaml") + var saasFile string + if tc.serviceName == "saas-managed-cluster-config" { + // osd-operator path structure + saasFile = filepath.Join(tmpDir, "data", "services", "osd-operators", "cicd", "saas", "saas-managed-cluster-config.yaml") + } else { + // Regular path structure + saasFile = filepath.Join(tmpDir, "data", "services", "test-service", "cicd", "saas", "saas-test-service.yaml") + } + _ = os.MkdirAll(filepath.Dir(saasFile), 0o755) _ = os.WriteFile(saasFile, []byte("dummy content"), 0o600) mockExec := new(MockExec) diff --git a/cmd/promote/pathutil/pathutil.go b/cmd/promote/pathutil/pathutil.go new file mode 100644 index 000000000..0bc9c02df --- /dev/null +++ b/cmd/promote/pathutil/pathutil.go @@ -0,0 +1,88 @@ +package pathutil + +import ( + "fmt" + "os" + "path/filepath" + "strings" +) + +// DeriveAppYmlPath determines the location of app.yml for a given service. +func DeriveAppYmlPath(gitDirectory, saasFile, componentName string) (string, error) { + derivedPath, err := deriveFromSaasPath(gitDirectory, saasFile, componentName) + if err == nil { + if _, statErr := os.Stat(derivedPath); statErr == nil { + return derivedPath, nil + } + } + + // as fallback, searches filesystem for likely loccations + searchedPath, searchErr := searchFilesystem(gitDirectory, componentName) + if searchErr == nil { + return searchedPath, nil + } + + if err == nil { + return derivedPath, nil + } + + return "", fmt.Errorf("could not determine app.yml path for %s: derive error: %v, search error: %v", + componentName, err, searchErr) +} + +// deriveFromSaasPath derives the app.yml path by parsing the saasFile path structure. +func deriveFromSaasPath(gitDirectory, saasFile, componentName string) (string, error) { + // absolute saasFile path into relative path + relPath, err := filepath.Rel(gitDirectory, saasFile) + if err != nil { + return "", fmt.Errorf("failed to get relative path from %s to %s: %v", gitDirectory, saasFile, err) + } + + pathParts := strings.Split(relPath, string(filepath.Separator)) + var parentServiceDir string + + // Find "services" in the path and extract the next component + for i, part := range pathParts { + if part == "services" && i+1 < len(pathParts) { + parentServiceDir = pathParts[i+1] + break + } + } + + if parentServiceDir == "" { + return "", fmt.Errorf("invalid saasFile path: 'services' directory not found in %s", relPath) + } + + if parentServiceDir != componentName && parentServiceDir != "cicd" { + // Nested structure: data/services/{parent}/{component}/app.yml + return filepath.Join(gitDirectory, "data", "services", parentServiceDir, componentName, "app.yml"), nil + } + + // Flat structure; data/services/{component}/app.yml + return filepath.Join(gitDirectory, "data", "services", componentName, "app.yml"), nil +} + +// searchFilesystem searches for app.yml in specific "likely" paths +func searchFilesystem(gitDirectory, componentName string) (string, error) { + parentDirs := []string{ + "", // services/{component} + "osd-operators", // services/osd-operators/{component} + "backplane", // services/backplane/{component} + "configuration-anomaly-detection", + } + + for _, parent := range parentDirs { + var candidatePath string + if parent == "" { + candidatePath = filepath.Join(gitDirectory, "data", "services", componentName, "app.yml") + } else { + candidatePath = filepath.Join(gitDirectory, "data", "services", parent, componentName, "app.yml") + } + + if _, err := os.Stat(candidatePath); err == nil { + return candidatePath, nil + } + } + + return "", fmt.Errorf("app.yml not found for component %s in any known location", componentName) +} diff --git a/cmd/promote/pathutil/pathutil_test.go b/cmd/promote/pathutil/pathutil_test.go new file mode 100644 index 000000000..7e3f4b19f --- /dev/null +++ b/cmd/promote/pathutil/pathutil_test.go @@ -0,0 +1,279 @@ +package pathutil + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestDeriveAppYmlPath(t *testing.T) { + tests := []struct { + name string + setup func(t *testing.T) (gitDir, saasFile, component string) + expectedPath string + expectError bool + errorContains string + }{ + { + name: "flat_structure_file_exists", + setup: func(t *testing.T) (string, string, string) { + tmpDir := t.TempDir() + appYmlPath := filepath.Join(tmpDir, "data", "services", "my-service", "app.yml") + require.NoError(t, os.MkdirAll(filepath.Dir(appYmlPath), 0755)) + require.NoError(t, os.WriteFile(appYmlPath, []byte("test"), 0600)) + + saasFile := filepath.Join(tmpDir, "data", "services", "my-service", "cicd", "saas", "saas-my-service.yaml") + return tmpDir, saasFile, "my-service" + }, + expectedPath: "data/services/my-service/app.yml", + expectError: false, + }, + { + name: "nested_osd_operators_file_exists", + setup: func(t *testing.T) (string, string, string) { + tmpDir := t.TempDir() + appYmlPath := filepath.Join(tmpDir, "data", "services", "osd-operators", "managed-cluster-config", "app.yml") + require.NoError(t, os.MkdirAll(filepath.Dir(appYmlPath), 0755)) + require.NoError(t, os.WriteFile(appYmlPath, []byte("test"), 0600)) + + saasFile := filepath.Join(tmpDir, "data", "services", "osd-operators", "cicd", "saas", "saas-managed-cluster-config.yaml") + return tmpDir, saasFile, "managed-cluster-config" + }, + expectedPath: "data/services/osd-operators/managed-cluster-config/app.yml", + expectError: false, + }, + { + name: "nested_backplane_file_exists", + setup: func(t *testing.T) (string, string, string) { + tmpDir := t.TempDir() + appYmlPath := filepath.Join(tmpDir, "data", "services", "backplane", "backplane-api", "app.yml") + require.NoError(t, os.MkdirAll(filepath.Dir(appYmlPath), 0755)) + require.NoError(t, os.WriteFile(appYmlPath, []byte("test"), 0600)) + + saasFile := filepath.Join(tmpDir, "data", "services", "backplane", "cicd", "saas", "saas-backplane-api.yaml") + return tmpDir, saasFile, "backplane-api" + }, + expectedPath: "data/services/backplane/backplane-api/app.yml", + expectError: false, + }, + { + name: "derivation_fails_but_filesystem_search_succeeds", + setup: func(t *testing.T) (string, string, string) { + tmpDir := t.TempDir() + appYmlPath := filepath.Join(tmpDir, "data", "services", "osd-operators", "test-operator", "app.yml") + require.NoError(t, os.MkdirAll(filepath.Dir(appYmlPath), 0755)) + require.NoError(t, os.WriteFile(appYmlPath, []byte("test"), 0600)) + + saasFile := filepath.Join(tmpDir, "unusual", "path", "saas-test-operator.yaml") + return tmpDir, saasFile, "test-operator" + }, + expectedPath: "data/services/osd-operators/test-operator/app.yml", + expectError: false, + }, + { + name: "file_does_not_exist_returns_derived_path_anyway", + setup: func(t *testing.T) (string, string, string) { + tmpDir := t.TempDir() + saasFile := filepath.Join(tmpDir, "data", "services", "new-service", "cicd", "saas", "saas-new-service.yaml") + return tmpDir, saasFile, "new-service" + }, + expectedPath: "data/services/new-service/app.yml", + expectError: false, + }, + { + name: "completely_invalid_path_returns_error", + setup: func(t *testing.T) (string, string, string) { + tmpDir := t.TempDir() + saasFile := filepath.Join(tmpDir, "invalid", "path", "saas-test.yaml") + return tmpDir, saasFile, "test" + }, + expectError: true, + errorContains: "could not determine app.yml path", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gitDir, saasFile, component := tt.setup(t) + + result, err := DeriveAppYmlPath(gitDir, saasFile, component) + + if tt.expectError { + assert.Error(t, err) + if tt.errorContains != "" { + assert.Contains(t, err.Error(), tt.errorContains) + } + } else { + assert.NoError(t, err) + relPath, err := filepath.Rel(gitDir, result) + require.NoError(t, err) + assert.Equal(t, tt.expectedPath, relPath) + } + }) + } +} + +func TestDeriveFromSaasPath(t *testing.T) { + tests := []struct { + name string + gitDirectory string + saasFile string + componentName string + expectedPath string + expectError bool + errorContains string + }{ + { + name: "flat_structure", + gitDirectory: "/app-interface", + saasFile: "/app-interface/data/services/my-service/cicd/saas/saas-my-service.yaml", + componentName: "my-service", + expectedPath: "/app-interface/data/services/my-service/app.yml", + expectError: false, + }, + { + name: "nested_osd_operators", + gitDirectory: "/app-interface", + saasFile: "/app-interface/data/services/osd-operators/cicd/saas/saas-managed-cluster-config.yaml", + componentName: "managed-cluster-config", + expectedPath: "/app-interface/data/services/osd-operators/managed-cluster-config/app.yml", + expectError: false, + }, + { + name: "nested_backplane", + gitDirectory: "/app-interface", + saasFile: "/app-interface/data/services/backplane/cicd/saas/saas-backplane-api.yaml", + componentName: "backplane-api", + expectedPath: "/app-interface/data/services/backplane/backplane-api/app.yml", + expectError: false, + }, + { + name: "path_without_services_fails", + gitDirectory: "/app-interface", + saasFile: "/app-interface/data/other/cicd/saas/saas-test.yaml", + componentName: "test", + expectError: true, + errorContains: "services' directory not found", + }, + { + name: "cicd_parent_uses_flat_structure", + gitDirectory: "/app-interface", + saasFile: "/app-interface/data/services/cicd/saas/saas-test.yaml", + componentName: "test", + expectedPath: "/app-interface/data/services/test/app.yml", + expectError: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := deriveFromSaasPath(tt.gitDirectory, tt.saasFile, tt.componentName) + + if tt.expectError { + assert.Error(t, err) + if tt.errorContains != "" { + assert.Contains(t, err.Error(), tt.errorContains) + } + } else { + assert.NoError(t, err) + assert.Equal(t, tt.expectedPath, result) + } + }) + } +} + +func TestSearchFilesystem(t *testing.T) { + tests := []struct { + name string + setup func(t *testing.T) (gitDir, component string) + expectedPath string + expectError bool + errorContains string + }{ + { + name: "finds_flat_structure", + setup: func(t *testing.T) (string, string) { + tmpDir := t.TempDir() + appYmlPath := filepath.Join(tmpDir, "data", "services", "my-service", "app.yml") + require.NoError(t, os.MkdirAll(filepath.Dir(appYmlPath), 0755)) + require.NoError(t, os.WriteFile(appYmlPath, []byte("test"), 0600)) + return tmpDir, "my-service" + }, + expectedPath: "data/services/my-service/app.yml", + expectError: false, + }, + { + name: "finds_osd_operators", + setup: func(t *testing.T) (string, string) { + tmpDir := t.TempDir() + appYmlPath := filepath.Join(tmpDir, "data", "services", "osd-operators", "test-operator", "app.yml") + require.NoError(t, os.MkdirAll(filepath.Dir(appYmlPath), 0755)) + require.NoError(t, os.WriteFile(appYmlPath, []byte("test"), 0600)) + return tmpDir, "test-operator" + }, + expectedPath: "data/services/osd-operators/test-operator/app.yml", + expectError: false, + }, + { + name: "finds_backplane", + setup: func(t *testing.T) (string, string) { + tmpDir := t.TempDir() + appYmlPath := filepath.Join(tmpDir, "data", "services", "backplane", "backplane-api", "app.yml") + require.NoError(t, os.MkdirAll(filepath.Dir(appYmlPath), 0755)) + require.NoError(t, os.WriteFile(appYmlPath, []byte("test"), 0600)) + return tmpDir, "backplane-api" + }, + expectedPath: "data/services/backplane/backplane-api/app.yml", + expectError: false, + }, + { + name: "file_not_found_returns_error", + setup: func(t *testing.T) (string, string) { + tmpDir := t.TempDir() + return tmpDir, "nonexistent" + }, + expectError: true, + errorContains: "app.yml not found", + }, + { + name: "prefers_first_match", + setup: func(t *testing.T) (string, string) { + tmpDir := t.TempDir() + flatPath := filepath.Join(tmpDir, "data", "services", "duplicate", "app.yml") + require.NoError(t, os.MkdirAll(filepath.Dir(flatPath), 0755)) + require.NoError(t, os.WriteFile(flatPath, []byte("flat"), 0600)) + + nestedPath := filepath.Join(tmpDir, "data", "services", "osd-operators", "duplicate", "app.yml") + require.NoError(t, os.MkdirAll(filepath.Dir(nestedPath), 0755)) + require.NoError(t, os.WriteFile(nestedPath, []byte("nested"), 0600)) + + return tmpDir, "duplicate" + }, + expectedPath: "data/services/duplicate/app.yml", + expectError: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gitDir, component := tt.setup(t) + + result, err := searchFilesystem(gitDir, component) + + if tt.expectError { + assert.Error(t, err) + if tt.errorContains != "" { + assert.Contains(t, err.Error(), tt.errorContains) + } + } else { + assert.NoError(t, err) + relPath, err := filepath.Rel(gitDir, result) + require.NoError(t, err) + assert.Equal(t, tt.expectedPath, relPath) + } + }) + } +} diff --git a/cmd/promote/saas/utils.go b/cmd/promote/saas/utils.go index 264253203..ad60cde20 100644 --- a/cmd/promote/saas/utils.go +++ b/cmd/promote/saas/utils.go @@ -10,6 +10,7 @@ import ( "github.com/openshift/osdctl/cmd/promote/git" "github.com/openshift/osdctl/cmd/promote/iexec" + "github.com/openshift/osdctl/cmd/promote/pathutil" kyaml "sigs.k8s.io/kustomize/kyaml/yaml" ) @@ -83,7 +84,7 @@ func servicePromotion(appInterface git.AppInterface, serviceName, gitHash string } if hotfix { - err = updateAppYmlWithHotfix(appInterface, serviceName, promotionGitHash) + err = updateAppYmlWithHotfix(appInterface, serviceName, saasDir, promotionGitHash) if err != nil { return fmt.Errorf("failed to update app.yml with hotfix: %v", err) } @@ -222,10 +223,13 @@ func setHotfixVersion(fileContent string, componentName string, gitHash string) } // locates the corresponding app.yml file, and updates the file with the hotfix sha -func updateAppYmlWithHotfix(appInterface git.AppInterface, serviceName, gitHash string) error { +func updateAppYmlWithHotfix(appInterface git.AppInterface, serviceName, saasDir, gitHash string) error { componentName := strings.TrimPrefix(serviceName, "saas-") - appYmlPath := filepath.Join(appInterface.GitDirectory, "data", "services", componentName, "app.yml") + appYmlPath, err := pathutil.DeriveAppYmlPath(appInterface.GitDirectory, saasDir, componentName) + if err != nil { + return fmt.Errorf("failed to derive app.yml path: %v", err) + } if _, err := os.Stat(appYmlPath); os.IsNotExist(err) { return fmt.Errorf("app.yml file not found at %s", appYmlPath) diff --git a/cmd/promote/saas/utils_test.go b/cmd/promote/saas/utils_test.go index e59343494..4d8b34344 100644 --- a/cmd/promote/saas/utils_test.go +++ b/cmd/promote/saas/utils_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/openshift/osdctl/cmd/promote/git" + "github.com/openshift/osdctl/cmd/promote/pathutil" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -352,7 +353,7 @@ func TestUpdateAppYmlWithHotfix(t *testing.T) { errorSubstr string }{ { - name: "successfully_updates_app_yml", + name: "successfully_updates_app_yml_for_service_directly_under_services", setup: func(t *testing.T) (git.AppInterface, string) { tmpDir := t.TempDir() servicesDir := filepath.Join(tmpDir, "data", "services", "test-service") @@ -368,17 +369,68 @@ codeComponents: err = os.WriteFile(appYmlPath, []byte(appYmlContent), 0600) require.NoError(t, err) - return git.AppInterface{GitDirectory: tmpDir}, "saas-test-service" + saasDir := filepath.Join(tmpDir, "data", "services", "test-service", "cicd", "saas", "saas-test-service.yaml") + return git.AppInterface{GitDirectory: tmpDir}, saasDir }, serviceName: "saas-test-service", gitHash: "abc123", expectError: false, }, + { + name: "successfully_updates_app_yml_for_osd_operator", + setup: func(t *testing.T) (git.AppInterface, string) { + tmpDir := t.TempDir() + // imitating operators' directories + servicesDir := filepath.Join(tmpDir, "data", "services", "osd-operators", "managed-cluster-config") + err := os.MkdirAll(servicesDir, 0755) + require.NoError(t, err) + + appYmlContent := ` +codeComponents: + - name: managed-cluster-config + url: https://github.com/openshift/managed-cluster-config +` + appYmlPath := filepath.Join(servicesDir, "app.yml") + err = os.WriteFile(appYmlPath, []byte(appYmlContent), 0600) + require.NoError(t, err) + + saasDir := filepath.Join(tmpDir, "data", "services", "osd-operators", "cicd", "saas", "saas-managed-cluster-config.yaml") + return git.AppInterface{GitDirectory: tmpDir}, saasDir + }, + serviceName: "saas-managed-cluster-config", + gitHash: "7f2dbae9d4284f8b68813270c1202ca3435459e5", + expectError: false, + }, + { + name: "successfully_updates_app_yml_for_backplane_service", + setup: func(t *testing.T) (git.AppInterface, string) { + tmpDir := t.TempDir() + // Imitate dir structure for backplane + servicesDir := filepath.Join(tmpDir, "data", "services", "backplane", "backplane-api") + err := os.MkdirAll(servicesDir, 0755) + require.NoError(t, err) + + appYmlContent := ` +codeComponents: + - name: backplane-api + url: https://github.com/openshift/backplane-api +` + appYmlPath := filepath.Join(servicesDir, "app.yml") + err = os.WriteFile(appYmlPath, []byte(appYmlContent), 0600) + require.NoError(t, err) + saasDir := filepath.Join(tmpDir, "data", "services", "backplane", "cicd", "saas", "saas-backplane-api.yaml") + return git.AppInterface{GitDirectory: tmpDir}, saasDir + }, + serviceName: "saas-backplane-api", + gitHash: "def456", + expectError: false, + }, { name: "fails_when_app_yml_not_found", setup: func(t *testing.T) (git.AppInterface, string) { tmpDir := t.TempDir() - return git.AppInterface{GitDirectory: tmpDir}, "saas-nonexistent-service" + saasDir := filepath.Join(tmpDir, "data", "services", "cicd", "saas", "saas-nonexistent-service.yaml") + return git.AppInterface{GitDirectory: tmpDir}, saasDir }, serviceName: "saas-nonexistent-service", gitHash: "abc123", @@ -402,7 +454,8 @@ codeComponents: err = os.WriteFile(appYmlPath, []byte(appYmlContent), 0600) require.NoError(t, err) - return git.AppInterface{GitDirectory: tmpDir}, "saas-test-service" + saasDir := filepath.Join(tmpDir, "data", "services", "test-service", "cicd", "saas", "saas-test-service.yaml") + return git.AppInterface{GitDirectory: tmpDir}, saasDir }, serviceName: "saas-test-service", gitHash: "abc123", @@ -413,9 +466,9 @@ codeComponents: for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - appInterface, _ := tc.setup(t) + appInterface, saasDir := tc.setup(t) - err := updateAppYmlWithHotfix(appInterface, tc.serviceName, tc.gitHash) + err := updateAppYmlWithHotfix(appInterface, tc.serviceName, saasDir, tc.gitHash) if tc.expectError { assert.Error(t, err) @@ -423,8 +476,11 @@ codeComponents: } else { assert.NoError(t, err) + // Use the same path derivation logic as the implementation componentName := strings.TrimPrefix(tc.serviceName, "saas-") - appYmlPath := filepath.Join(appInterface.GitDirectory, "data", "services", componentName, "app.yml") + appYmlPath, err := pathutil.DeriveAppYmlPath(appInterface.GitDirectory, saasDir, componentName) + require.NoError(t, err) + content, readErr := os.ReadFile(appYmlPath) assert.NoError(t, readErr) assert.Contains(t, string(content), tc.gitHash)