feat: Add the ability to remove the files with GerritMergeRequest CR (#…

…30)

Change-Id: If9b6963174f52d8015d5e1e857c31adb73674830
(cherry picked from commit 5f9738c)

.golangci.yaml

@@ -428,15 +428,6 @@ linters-settings:
     enable-all: false
     disable-all: false
 
-  depguard:
-    list-type: blacklist
-    include-go-root: false
-    packages:
-      - github.com/sirupsen/logrus
-    packages-with-error-message:
-      # specify an error message to output when a blacklisted package is used
-      - github.com/sirupsen/logrus: "logging is allowed only by logutils.Log"
-
   ifshort:
     # Maximum length of variable declaration measured in number of lines, after which linter won't suggest using short syntax.
     # Has higher priority than max-decl-chars.
@@ -882,7 +873,6 @@ linters:
     - containedctx
     - cyclop
     - decorder
-    - depguard
     - dogsled
     - dupl
     - durationcheck

Dockerfile

@@ -7,7 +7,7 @@ ENV OPERATOR=/usr/local/bin/gerrit-operator \
 
 RUN apk add --no-cache ca-certificates==20240226-r0 \
                        openssh-client==9.3_p2-r1 \
-                       openssl==3.1.4-r5 \
+                       openssl==3.1.4-r6 \
                        git==2.40.1-r0
 
 # install operator binary

api/v1/gerritmergerequest_types.go

@@ -49,7 +49,9 @@ type GerritMergeRequestSpec struct {
 	CommitMessage string `json:"commitMessage,omitempty"`
 
 	// ChangesConfigMap is the name of the ConfigMap, which contains files contents that should be merged.
-	// ConfigMap should eny data keys with content in the json format: {"path": "/controllers/user.go", "contents": "some code here"}.
+	// ConfigMap should contain eny data keys with content in the json
+	// format: {"path": "/controllers/user.go", "contents": "some code here"} - to add file
+	// or format: {"path": "/controllers/user.go"} - to remove file.
 	// If files already exist in the project, they will be overwritten.
 	// If empty, sourceBranch should be set.
 	// +optional

config/crd/bases/v2.edp.epam.com_gerritmergerequests.yaml

@@ -55,9 +55,11 @@ spec:
               changesConfigMap:
                 description: 'ChangesConfigMap is the name of the ConfigMap, which
                   contains files contents that should be merged. ConfigMap should
-                  eny data keys with content in the json format: {"path": "/controllers/user.go",
-                  "contents": "some code here"}. If files already exist in the project,
-                  they will be overwritten. If empty, sourceBranch should be set.'
+                  contain eny data keys with content in the json format: {"path":
+                  "/controllers/user.go", "contents": "some code here"} - to add file
+                  or format: {"path": "/controllers/user.go"} - to remove file. If
+                  files already exist in the project, they will be overwritten. If
+                  empty, sourceBranch should be set.'
                 example: config-map-new-feature
                 type: string
               commitMessage:

controllers/merge_request/controller.go

@@ -57,6 +57,7 @@ type GitClient interface {
 	CheckoutBranch(projectName, branch string) error
 	Commit(projectName, message string, files []string, user *git.User) error
 	SetFileContents(projectName, filePath, contents string) error
+	RemoveFile(projectName, filePath string) (bool, error)
 }
 
 type GerritClient interface {
@@ -65,8 +66,8 @@ type GerritClient interface {
 }
 
 type MRConfigMapFile struct {
-	Path     string `json:"path"`
-	Contents string `json:"contents"`
+	Path     string  `json:"path"`
+	Contents *string `json:"contents"`
 }
 
 func NewReconcile(k8sClient client.Client, log logr.Logger,
@@ -302,11 +303,24 @@ func (r *Reconcile) commitFiles(ctx context.Context, instance *gerritApi.GerritM
 			return errors.Wrap(err, "unable to decode file")
 		}
 
-		if err := gitClient.SetFileContents(instance.Spec.ProjectName, mrFile.Path, mrFile.Contents); err != nil {
-			return errors.Wrap(err, "unable to set file contents")
+		fileChanged := true
+
+		if mrFile.Contents != nil {
+			if err := gitClient.SetFileContents(instance.Spec.ProjectName, mrFile.Path, *mrFile.Contents); err != nil {
+				return errors.Wrap(err, "unable to set file contents")
+			}
+		} else {
+			var err error
+
+			fileChanged, err = gitClient.RemoveFile(instance.Spec.ProjectName, mrFile.Path)
+			if err != nil {
+				return errors.Wrap(err, "unable to remove file")
+			}
 		}
 
-		addFiles = append(addFiles, mrFile.Path)
+		if fileChanged {
+			addFiles = append(addFiles, mrFile.Path)
+		}
 	}
 
 	gitUser := &git.User{Name: instance.Spec.AuthorName, Email: instance.Spec.AuthorEmail}

controllers/merge_request/controller_test.go

@@ -9,6 +9,7 @@ import (
 
 	"github.com/go-logr/logr"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
 	coreV1 "k8s.io/api/core/v1"
@@ -228,9 +229,15 @@ func (s *ControllerTestSuite) TestConfigMap() {
 	err := coreV1.AddToScheme(s.scheme)
 	require.NoError(s.T(), err)
 
-	cm := coreV1.ConfigMap{Data: map[string]string{"test.txt": `{"path": "test.txt", "contents": "test"}`}, ObjectMeta: metaV1.ObjectMeta{
-		Name: s.mergeRequest.Spec.ChangesConfigMap, Namespace: s.mergeRequest.Namespace,
-	}}
+	cm := coreV1.ConfigMap{
+		ObjectMeta: metaV1.ObjectMeta{
+			Name: s.mergeRequest.Spec.ChangesConfigMap, Namespace: s.mergeRequest.Namespace,
+		},
+		Data: map[string]string{
+			"create file": `{"path": "test.txt", "contents": "test"}`,
+			"remove file": `{"path": "test2.txt"}`,
+		},
+	}
 
 	fakeClient := fake.NewClientBuilder().WithScheme(s.scheme).
 		WithRuntimeObjects(s.rootGerrit, s.mergeRequest, &cm).Build()
@@ -243,8 +250,14 @@ func (s *ControllerTestSuite) TestConfigMap() {
 	s.gitClient.On("CheckoutBranch", s.mergeRequest.Spec.ProjectName, s.mergeRequest.TargetBranch()).
 		Return(nil)
 	s.gitClient.On("SetFileContents", s.mergeRequest.Spec.ProjectName, "test.txt", "test").Return(nil)
-	s.gitClient.On("Commit", s.mergeRequest.Spec.ProjectName, commitMessage(s.mergeRequest.CommitMessage(),
-		changeID), []string{"test.txt"},
+	s.gitClient.On("RemoveFile", s.mergeRequest.Spec.ProjectName, "test2.txt").Return(true, nil)
+	s.gitClient.On("Commit",
+		s.mergeRequest.Spec.ProjectName,
+		commitMessage(s.mergeRequest.CommitMessage(), changeID),
+		mock.MatchedBy(func(files []string) bool {
+			return assert.Contains(s.T(), files, "test.txt") &&
+				assert.Contains(s.T(), files, "test2.txt")
+		}),
 		&git.User{Name: s.mergeRequest.Spec.AuthorName, Email: s.mergeRequest.Spec.AuthorEmail}).Return(nil)
 
 	rec := Reconcile{

deploy-templates/crds/v2.edp.epam.com_gerritmergerequests.yaml

@@ -55,9 +55,11 @@ spec:
               changesConfigMap:
                 description: 'ChangesConfigMap is the name of the ConfigMap, which
                   contains files contents that should be merged. ConfigMap should
-                  eny data keys with content in the json format: {"path": "/controllers/user.go",
-                  "contents": "some code here"}. If files already exist in the project,
-                  they will be overwritten. If empty, sourceBranch should be set.'
+                  contain eny data keys with content in the json format: {"path":
+                  "/controllers/user.go", "contents": "some code here"} - to add file
+                  or format: {"path": "/controllers/user.go"} - to remove file. If
+                  files already exist in the project, they will be overwritten. If
+                  empty, sourceBranch should be set.'
                 example: config-map-new-feature
                 type: string
               commitMessage:

docs/api.md

@@ -400,7 +400,7 @@ GerritMergeRequestSpec defines the desired state of GerritMergeRequest.
         <td><b>changesConfigMap</b></td>
         <td>string</td>
         <td>
-          ChangesConfigMap is the name of the ConfigMap, which contains files contents that should be merged. ConfigMap should eny data keys with content in the json format: {"path": "/controllers/user.go", "contents": "some code here"}. If files already exist in the project, they will be overwritten. If empty, sourceBranch should be set.<br/>
+          ChangesConfigMap is the name of the ConfigMap, which contains files contents that should be merged. ConfigMap should contain eny data keys with content in the json format: {"path": "/controllers/user.go", "contents": "some code here"} - to add file or format: {"path": "/controllers/user.go"} - to remove file. If files already exist in the project, they will be overwritten. If empty, sourceBranch should be set.<br/>
         </td>
         <td>false</td>
       </tr><tr>

pkg/client/git/client.go

@@ -66,6 +66,22 @@ func (c *Client) SetFileContents(projectName, filePath, contents string) error {
 	return nil
 }
 
+func (c *Client) RemoveFile(projectName, filePath string) (bool, error) {
+	projectPath := c.projectPath(projectName)
+	filePath = path.Join(projectPath, filePath)
+
+	err := os.Remove(filePath)
+	if err != nil {
+		if !os.IsNotExist(err) {
+			return false, errors.Wrapf(err, "unable to remove file: %s", filePath)
+		}
+
+		return false, nil
+	}
+
+	return true, nil
+}
+
 func (c *Client) Clone(projectName string) (projectPath string, err error) {
 	projectPath = c.projectPath(projectName)
 	_, err = git.PlainClone(

pkg/client/git/client_test.go

@@ -204,3 +204,52 @@ func removeAllWithErrCapture(t *testing.T, p string) {
 	err := os.RemoveAll(p)
 	require.NoError(t, err)
 }
+
+func TestClient_RemoveFile(t *testing.T) {
+	t.Parallel()
+
+	tmp := t.TempDir()
+	projectDir := path.Join(tmp, "test")
+	require.NoError(t, os.MkdirAll(projectDir, 0o777))
+
+	tests := []struct {
+		name     string
+		filePath string
+		prepare  func(t *testing.T)
+		want     bool
+		wantErr  require.ErrorAssertionFunc
+	}{
+		{
+			name:     "file exists",
+			filePath: "test.txt",
+			prepare: func(t *testing.T) {
+				_, err := os.Create(path.Join(projectDir, "test.txt"))
+				require.NoError(t, err)
+			},
+			want:    true,
+			wantErr: require.NoError,
+		},
+		{
+			name:     "file does not exist",
+			filePath: "test.txt",
+			prepare: func(t *testing.T) {
+			},
+			want:    false,
+			wantErr: require.NoError,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			tt.prepare(t)
+
+			c := &Client{
+				workingDir: tmp,
+			}
+
+			got, err := c.RemoveFile("test", tt.filePath)
+			tt.wantErr(t, err)
+			assert.Equal(t, tt.want, got)
+		})
+	}
+}

sonar-project.properties

@@ -3,4 +3,4 @@ sonar.projectName=gerrit-operator
 sonar.projectVersion=1.0
 sonar.go.coverage.reportPaths=coverage.out
 sonar.test.inclusions=**/*_test.go
-sonar.exclusions=**/deploy-templates/**,api/edp/v1alpha1/*,**/main.go,**/*.groovy,**/.github/**,**/mock_*.go,**/mocks/**,**/mock/**,**/*_types.go,**/factory.go,**/config/**,**/build/**,Dockerfile
+sonar.exclusions=**/deploy-templates/**,**/*generated.*.go,**/main.go,**/*.groovy,**/.github/**,**/mock_*.go,**/mocks/**,**/mock/**,**/*_types.go,**/factory.go,**/config/**,**/build/**,Dockerfile