fix(shipyard-controller): Do not validate gitCredentials when not set…

… during project update (#8939)

Signed-off-by: odubajDT <ondrej.dubaj@dynatrace.com>

shipyard-controller/internal/handler/projecthandler.go

@@ -133,12 +133,8 @@ func (p ProjectValidator) validateUpdateProjectParams(updateProjectParams *model
 		}
 	}
 
-	if p.AutomaticProvisioningURL != "" && updateProjectParams.GitCredentials == nil {
-		return nil
-	}
-
 	if updateProjectParams.GitCredentials == nil {
-		return fmt.Errorf("gitCredentials cannot be empty")
+		return nil
 	}
 
 	if err := common.ValidateGitRemoteURL(updateProjectParams.GitCredentials.RemoteURL); err != nil {
@@ -389,9 +385,11 @@ func (ph *ProjectHandler) UpdateProject(c *gin.Context) {
 		return
 	}
 
-	if err := ph.RemoteURLValidator.Validate(params.GitCredentials.RemoteURL); err != nil {
-		SetUnprocessableEntityResponse(c, fmt.Sprintf(common.InvalidRemoteURLMsg, params.GitCredentials.RemoteURL))
-		return
+	if params.GitCredentials != nil && params.GitCredentials.RemoteURL != "" {
+		if err := ph.RemoteURLValidator.Validate(params.GitCredentials.RemoteURL); err != nil {
+			SetUnprocessableEntityResponse(c, fmt.Sprintf(common.InvalidRemoteURLMsg, params.GitCredentials.RemoteURL))
+			return
+		}
 	}
 
 	common.LockProject(*params.Name)

shipyard-controller/internal/handler/projecthandler_test.go

@@ -705,7 +705,7 @@ func TestUpdateProject(t *testing.T) {
 			expectedHTTPStatus: http.StatusOK,
 		},
 		{
-			name: "Update project with validator failed",
+			name: "Update project with remoteURL validator failed",
 			fields: fields{
 				ProjectManager: &fake.IProjectManagerMock{
 					UpdateFunc: func(params *models.UpdateProjectParams) (error, common.RollbackFunc) {
@@ -743,14 +743,10 @@ func TestUpdateProject(t *testing.T) {
 				},
 				EnvConfig:             config.EnvConfig{ProjectNameMaxSize: 200},
 				RepositoryProvisioner: &fake2.IRepositoryProvisionerMock{},
-				RemoteURLValidator: fake2.RequestValidatorMock{
-					ValidateFunc: func(url string) error {
-						return fmt.Errorf("some err")
-					},
-				},
+				RemoteURLValidator:    remoteURLValidator,
 			},
 			jsonPayload:        examplePayload2,
-			expectedHTTPStatus: http.StatusBadRequest,
+			expectedHTTPStatus: http.StatusOK,
 		},
 		{
 			name: "Update project with invalid token",
@@ -1233,11 +1229,11 @@ func Test_ProjectValidator_UpdateParams(t *testing.T) {
 			provisioningURL: "some-url",
 		},
 		{
-			name: "invalid params",
+			name: "valid params #2",
 			params: models.UpdateProjectParams{
 				Name: &projectName,
 			},
-			wantErr:         true,
+			wantErr:         false,
 			provisioningURL: "",
 		},
 		{

shipyard-controller/internal/handler/projectmanager.go

@@ -288,7 +288,9 @@ func (pm *ProjectManager) Update(params *models.UpdateProjectParams) (error, com
 	// copy by value
 
 	updateProject := *oldProject
-	updateProject.GitCredentials = toSecureGitCredentials(params.GitCredentials)
+	if params.GitCredentials != nil {
+		updateProject.GitCredentials = toSecureGitCredentials(params.GitCredentials)
+	}
 	if params.GitCredentials != nil && params.GitCredentials.RemoteURL != "" {
 		updateProject.IsUpstreamAutoProvisioned = false
 	}

shipyard-controller/internal/handler/projectmanager_test.go

@@ -1387,6 +1387,97 @@ func TestUpdate_FromProvisionedRepository(t *testing.T) {
 	assert.Equal(t, expectedUpdateShipyardResourceData, configStore.UpdateProjectResourceCalls()[0].Resource)
 }
 
+func TestUpdate_NoCredentialsFromProvisioned(t *testing.T) {
+
+	secretStore := &fake.SecretStoreMock{}
+	projectMVRepo := &db_mock.ProjectMVRepoMock{}
+	eventRepo := &db_mock.EventRepoMock{}
+	configStore := &common_mock.ConfigurationStoreMock{}
+	sequenceQueueRepo := &db_mock.SequenceQueueRepoMock{}
+	eventQueueRepo := &db_mock.EventQueueRepoMock{}
+	sequenceExecutionRepo := &db_mock.SequenceExecutionRepoMock{}
+
+	oldSecretsData, _ := json.Marshal(apimodels.GitAuthCredentials{
+		User: "provisioned-user",
+		HttpsAuth: &apimodels.HttpsGitAuth{
+			Token: "my-provisioned-token",
+		},
+		RemoteURL: "http://provisioned.url",
+	})
+
+	gitCredentials := apimodels.GitAuthCredentialsSecure{
+		RemoteURL: "http://provisioned.url",
+		User:      "provisioned-user",
+	}
+
+	oldProjectData := &apimodels.ExpandedProject{
+		CreationDate:              "old-creationdate",
+		GitCredentials:            &gitCredentials,
+		ProjectName:               "my-project",
+		Shipyard:                  "",
+		ShipyardVersion:           "v1",
+		IsUpstreamAutoProvisioned: true,
+	}
+
+	secretStore.GetSecretFunc = func(name string) (map[string][]byte, error) {
+
+		return map[string][]byte{"git-credentials": oldSecretsData}, nil
+	}
+
+	secretStore.UpdateSecretFunc = func(name string, content map[string][]byte) error {
+		return nil
+	}
+	projectMVRepo.GetProjectFunc = func(projectName string) (*apimodels.ExpandedProject, error) {
+		return oldProjectData, nil
+	}
+
+	configStore.UpdateProjectFunc = func(project apimodels.Project) error {
+		return nil
+	}
+
+	configStore.UpdateProjectResourceFunc = func(projectName string, resource *apimodels.Resource) error {
+		return nil
+	}
+
+	projectMVRepo.UpdateProjectFunc = func(prj *apimodels.ExpandedProject) error {
+		return nil
+	}
+
+	instance := NewProjectManager(configStore, secretStore, projectMVRepo, sequenceExecutionRepo, eventRepo, sequenceQueueRepo, eventQueueRepo)
+	myShipyard := "my-shipyard"
+
+	params := &models.UpdateProjectParams{
+		GitCredentials: nil,
+		Name:           common.Stringp("my-project"),
+		Shipyard:       &myShipyard,
+	}
+	err, rollback := instance.Update(params)
+	assert.Nil(t, err)
+	rollback()
+
+	projectUpdateData := apimodels.Project{
+		ProjectName: *params.Name,
+	}
+
+	projectDBUpdateData := &apimodels.ExpandedProject{
+		CreationDate:              "old-creationdate",
+		GitCredentials:            &gitCredentials,
+		ProjectName:               "my-project",
+		Shipyard:                  "my-shipyard",
+		ShipyardVersion:           "v1",
+		IsUpstreamAutoProvisioned: true,
+	}
+
+	expectedUpdateShipyardResourceData := &apimodels.Resource{
+		ResourceContent: *params.Shipyard,
+		ResourceURI:     common.Stringp("shipyard.yaml")}
+
+	assert.Equal(t, projectUpdateData, configStore.UpdateProjectCalls()[0].Project)
+	assert.Len(t, secretStore.UpdateSecretCalls(), 0)
+	assert.Equal(t, projectDBUpdateData, projectMVRepo.UpdateProjectCalls()[0].Prj)
+	assert.Equal(t, expectedUpdateShipyardResourceData, configStore.UpdateProjectResourceCalls()[0].Resource)
+}
+
 func TestUpdate_ShouldWorkWithEmptyGitUser(t *testing.T) {
 
 	secretStore := &fake.SecretStoreMock{}