feat: add gzip or none-gzipped patch sources (#474)

## Description  Please include a summary of the changes and the related issue. Please also include relevant motivation and context. List any dependencies that are required for this change. ## What type of PR is this? (check all applicable) - [ ] 🍕 Feature - [ ] 🐛 Bug Fix - [ ] 📝 Documentation Update - [ ] 🎨 Style - [ ] 🧑‍💻 Code Refactor - [ ] 🔥 Performance Improvements - [ ] ✅ Test - [ ] 🤖 Build - [ ] 🔁 CI - [ ] 📦 Chore (Release) - [ ] ⏩ Revert ## Related Tickets & Documents  - Related Issue # (issue) - Closes # (issue) - Fixes # (issue) > Remove if not applicable ## Screenshots  ## Added tests? - [ ] 👍 yes - [ ] 🙅 no, because they aren't needed - [ ] 🙋 no, because I need help - [ ] Separate ticket for tests # (issue/pr) Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration ## Added to documentation? - [ ] 📜 README.md - [ ] 🙅 no documentation needed ## Checklist: - [ ] My code follows the style guidelines of this project - [ ] I have performed a self-review of my code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes - [ ] Any dependent changes have been merged and published in downstream modules
open-component-model · Jun 19, 2024 · 42885d2 · 42885d2
1 parent e39728a
commit 42885d2
Show file tree

Hide file tree

Showing 8 changed files with 415 additions and 147 deletions.
diff --git a/.golangci.yaml b/.golangci.yaml
@@ -3,7 +3,6 @@ run:
   timeout: 10m
   tests: false
   allow-parallel-runners: true
-
 linters-settings:
   funlen:
     lines: 150
@@ -37,6 +36,7 @@ issues:
   exclude-files:
     - pkg/ocm/fakes/fakes.go
     - pkg/cache/fakes/cache.go
+    - pkg/untar/untar.go
   max-same-issues: 0
   max-issues-per-linter: 0
   exclude-rules:

diff --git a/controllers/configuration_controller_test.go b/controllers/configuration_controller_test.go
@@ -1355,164 +1355,182 @@ func TestConfigurationShouldReconcile(t *testing.T) {
 }
 
 func TestPatchStrategicMergeWithResourceSource(t *testing.T) {
-	cv := DefaultComponent.DeepCopy()
-	cv.Status.ComponentDescriptor = v1alpha1.Reference{
-		Name:    "test-component",
-		Version: "v0.0.1",
-		ComponentDescriptorRef: meta.NamespacedObjectReference{
-			Name:      cv.Name + "-descriptor",
-			Namespace: cv.Namespace,
+	testcases := []struct {
+		name     string
+		fileName string
+	}{
+		{
+			name:     "should patch with gzip header",
+			fileName: "git-repo.tar.gz",
+		},
+		{
+			name:     "should patch with plain tar header",
+			fileName: "git-repo.tar",
 		},
 	}
-	conditions.MarkTrue(cv, meta.ReadyCondition, meta.SucceededReason, "test")
 
-	cd := DefaultComponentDescriptor.DeepCopy()
+	for i, tt := range testcases {
+		t.Run(fmt.Sprintf("%d: %s", i, tt.name), func(t *testing.T) {
+			cv := DefaultComponent.DeepCopy()
+			cv.Status.ComponentDescriptor = v1alpha1.Reference{
+				Name:    "test-component",
+				Version: "v0.0.1",
+				ComponentDescriptorRef: meta.NamespacedObjectReference{
+					Name:      cv.Name + "-descriptor",
+					Namespace: cv.Namespace,
+				},
+			}
+			conditions.MarkTrue(cv, meta.ReadyCondition, meta.SucceededReason, "test")
 
-	resource := DefaultResource.DeepCopy()
-	patchResourceSource := DefaultResource.DeepCopy()
-	patchResourceSource.Name = "patch-test-resource"
+			cd := DefaultComponentDescriptor.DeepCopy()
 
-	name := "test-snapshot"
-	identity := ocmmetav1.Identity{
-		v1alpha1.ComponentNameKey:    cv.Status.ComponentDescriptor.ComponentDescriptorRef.Name,
-		v1alpha1.ComponentVersionKey: cv.Status.ComponentDescriptor.Version,
-		v1alpha1.ResourceNameKey:     resource.Spec.SourceRef.ResourceRef.Name,
-		v1alpha1.ResourceVersionKey:  resource.Spec.SourceRef.ResourceRef.Version,
-	}
-	snapshot := &v1alpha1.Snapshot{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      name,
-			Namespace: cv.Namespace,
-		},
-		Spec: v1alpha1.SnapshotSpec{
-			Identity: identity,
-		},
-	}
-	patchSnapshotSource := &v1alpha1.Snapshot{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      "patch-" + name,
-			Namespace: cv.Namespace,
-		},
-		Spec: v1alpha1.SnapshotSpec{
-			Identity: ocmmetav1.Identity{
+			resource := DefaultResource.DeepCopy()
+			patchResourceSource := DefaultResource.DeepCopy()
+			patchResourceSource.Name = "patch-test-resource"
+
+			name := "test-snapshot"
+			identity := ocmmetav1.Identity{
 				v1alpha1.ComponentNameKey:    cv.Status.ComponentDescriptor.ComponentDescriptorRef.Name,
 				v1alpha1.ComponentVersionKey: cv.Status.ComponentDescriptor.Version,
-				v1alpha1.ResourceNameKey:     "patch-snapshot-source",
+				v1alpha1.ResourceNameKey:     resource.Spec.SourceRef.ResourceRef.Name,
 				v1alpha1.ResourceVersionKey:  resource.Spec.SourceRef.ResourceRef.Version,
-			},
-		},
-	}
+			}
+			snapshot := &v1alpha1.Snapshot{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      name,
+					Namespace: cv.Namespace,
+				},
+				Spec: v1alpha1.SnapshotSpec{
+					Identity: identity,
+				},
+			}
+			patchSnapshotSource := &v1alpha1.Snapshot{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "patch-" + name,
+					Namespace: cv.Namespace,
+				},
+				Spec: v1alpha1.SnapshotSpec{
+					Identity: ocmmetav1.Identity{
+						v1alpha1.ComponentNameKey:    cv.Status.ComponentDescriptor.ComponentDescriptorRef.Name,
+						v1alpha1.ComponentVersionKey: cv.Status.ComponentDescriptor.Version,
+						v1alpha1.ResourceNameKey:     "patch-snapshot-source",
+						v1alpha1.ResourceVersionKey:  resource.Spec.SourceRef.ResourceRef.Version,
+					},
+				},
+			}
 
-	patchResourceSource.Status.SnapshotName = patchSnapshotSource.Name
-	resource.Status.SnapshotName = name
-	conditions.MarkTrue(resource, meta.ReadyCondition, meta.SucceededReason, "test")
-	conditions.MarkTrue(patchResourceSource, meta.ReadyCondition, meta.SucceededReason, "test")
-
-	source := v1alpha1.ObjectReference{
-		NamespacedObjectKindReference: meta.NamespacedObjectKindReference{
-			APIVersion: v1alpha1.GroupVersion.String(),
-			Kind:       "Resource",
-			Name:       "test-resource",
-			Namespace:  snapshot.Namespace,
-		},
-	}
+			patchResourceSource.Status.SnapshotName = patchSnapshotSource.Name
+			resource.Status.SnapshotName = name
+			conditions.MarkTrue(resource, meta.ReadyCondition, meta.SucceededReason, "test")
+			conditions.MarkTrue(patchResourceSource, meta.ReadyCondition, meta.SucceededReason, "test")
+
+			source := v1alpha1.ObjectReference{
+				NamespacedObjectKindReference: meta.NamespacedObjectKindReference{
+					APIVersion: v1alpha1.GroupVersion.String(),
+					Kind:       "Resource",
+					Name:       "test-resource",
+					Namespace:  snapshot.Namespace,
+				},
+			}
 
-	configuration := DefaultConfiguration.DeepCopy()
-	configuration.Spec.SourceRef = source
-	configuration.Spec.ConfigRef = nil
-	configuration.Status.SnapshotName = "configuration-snapshot"
-	configuration.Spec.PatchStrategicMerge = &v1alpha1.PatchStrategicMerge{
-		Source: v1alpha1.PatchStrategicMergeSource{
-			SourceRef: meta.NamespacedObjectKindReference{
-				APIVersion: v1alpha1.GroupVersion.String(),
-				Kind:       "Resource",
-				Name:       patchResourceSource.Name,
-				Namespace:  patchResourceSource.Namespace,
-			},
-			Path: "sites/eu-west-1/deployment.yaml",
-		},
-		Target: v1alpha1.PatchStrategicMergeTarget{
-			Path: "merge-target/merge-target.yaml",
-		},
-	}
+			configuration := DefaultConfiguration.DeepCopy()
+			configuration.Spec.SourceRef = source
+			configuration.Spec.ConfigRef = nil
+			configuration.Status.SnapshotName = "configuration-snapshot"
+			configuration.Spec.PatchStrategicMerge = &v1alpha1.PatchStrategicMerge{
+				Source: v1alpha1.PatchStrategicMergeSource{
+					SourceRef: meta.NamespacedObjectKindReference{
+						APIVersion: v1alpha1.GroupVersion.String(),
+						Kind:       "Resource",
+						Name:       patchResourceSource.Name,
+						Namespace:  patchResourceSource.Namespace,
+					},
+					Path: "sites/eu-west-1/deployment.yaml",
+				},
+				Target: v1alpha1.PatchStrategicMergeTarget{
+					Path: "merge-target/merge-target.yaml",
+				},
+			}
 
-	objs := []client.Object{cv, cd, resource, patchResourceSource, configuration}
-	conditions.MarkTrue(snapshot, meta.ReadyCondition, meta.SucceededReason, "test")
-	conditions.MarkTrue(patchSnapshotSource, meta.ReadyCondition, meta.SucceededReason, "test")
-	objs = append(objs, snapshot, patchSnapshotSource)
-
-	client := env.FakeKubeClient(WithObjects(objs...), WithAddToScheme(sourcev1.AddToScheme))
-	dynClient := env.FakeDynamicKubeClient(WithObjects(objs...))
-	cache := &cachefakes.FakeCache{}
-	snapshotWriter := ocmsnapshot.NewOCIWriter(client, cache, env.scheme)
-	fakeOcm := &fakes.MockFetcher{}
-	recorder := record.NewFakeRecorder(32)
-	content, err := os.Open(filepath.Join("testdata", "merge-target.tar.gz"))
-	require.NoError(t, err)
-	patchContent, err := os.Open(filepath.Join("testdata", "git-repo.tar.gz"))
-	require.NoError(t, err)
-	cache.FetchDataByDigestReturnsOnCall(0, content, nil)
-	cache.FetchDataByDigestReturnsOnCall(1, patchContent, nil)
-	fakeOcm.GetResourceReturnsOnCall(0, nil, nil)
-	fakeOcm.GetResourceReturnsOnCall(1, nil, nil)
-
-	cr := ConfigurationReconciler{
-		Client:        client,
-		DynamicClient: dynClient,
-		Scheme:        env.scheme,
-		EventRecorder: recorder,
-		MutationReconciler: MutationReconcileLooper{
-			Client:         client,
-			DynamicClient:  dynClient,
-			Scheme:         env.scheme,
-			OCMClient:      fakeOcm,
-			Cache:          cache,
-			SnapshotWriter: snapshotWriter,
-		},
-	}
+			objs := []client.Object{cv, cd, resource, patchResourceSource, configuration}
+			conditions.MarkTrue(snapshot, meta.ReadyCondition, meta.SucceededReason, "test")
+			conditions.MarkTrue(patchSnapshotSource, meta.ReadyCondition, meta.SucceededReason, "test")
+			objs = append(objs, snapshot, patchSnapshotSource)
 
-	t.Log("reconciling configuration")
-	_, err = cr.Reconcile(context.Background(), ctrl.Request{
-		NamespacedName: types.NamespacedName{
-			Namespace: configuration.Namespace,
-			Name:      configuration.Name,
-		},
-	})
-	require.NoError(t, err)
-
-	getErr := client.Get(context.Background(), types.NamespacedName{
-		Namespace: configuration.Namespace,
-		Name:      configuration.Name,
-	}, configuration)
-	require.NoError(t, getErr)
-
-	snapshotOutput := &v1alpha1.Snapshot{}
-	err = client.Get(context.Background(), types.NamespacedName{
-		Namespace: configuration.Namespace,
-		Name:      configuration.Status.SnapshotName,
-	}, snapshotOutput)
-
-	t.Log("verifying that the strategic merge was performed")
-	args := cache.PushDataCallingArgumentsOnCall(0)
-	data := args.Content
-	sourceFile := extractFileFromTarGz(t, io.NopCloser(bytes.NewBuffer([]byte(data))), "merge-target.yaml")
-	deployment := appsv1.Deployment{}
-	err = yaml.Unmarshal(sourceFile, &deployment)
-	assert.NoError(t, err)
-	assert.Equal(t, int32(2), *deployment.Spec.Replicas, "has correct number of replicas")
-	assert.Equal(t, 2, len(deployment.Spec.Template.Spec.Containers), "has correct number of containers")
-	assert.Equal(t, corev1.PullPolicy("Always"), deployment.Spec.Template.Spec.Containers[0].ImagePullPolicy)
-
-	close(recorder.Events)
-	event := ""
-	for e := range recorder.Events {
-		if strings.Contains(e, "Reconciliation finished, next run in") {
-			event = e
+			client := env.FakeKubeClient(WithObjects(objs...), WithAddToScheme(sourcev1.AddToScheme))
+			dynClient := env.FakeDynamicKubeClient(WithObjects(objs...))
+			cache := &cachefakes.FakeCache{}
+			snapshotWriter := ocmsnapshot.NewOCIWriter(client, cache, env.scheme)
+			fakeOcm := &fakes.MockFetcher{}
+			recorder := record.NewFakeRecorder(32)
+			content, err := os.Open(filepath.Join("testdata", "merge-target.tar.gz"))
+			require.NoError(t, err)
+			patchContent, err := os.Open(filepath.Join("testdata", tt.fileName))
+			require.NoError(t, err)
+			cache.FetchDataByDigestReturnsOnCall(0, content, nil)
+			cache.FetchDataByDigestReturnsOnCall(1, patchContent, nil)
+			fakeOcm.GetResourceReturnsOnCall(0, nil, nil)
+			fakeOcm.GetResourceReturnsOnCall(1, nil, nil)
 
-			break
-		}
+			cr := ConfigurationReconciler{
+				Client:        client,
+				DynamicClient: dynClient,
+				Scheme:        env.scheme,
+				EventRecorder: recorder,
+				MutationReconciler: MutationReconcileLooper{
+					Client:         client,
+					DynamicClient:  dynClient,
+					Scheme:         env.scheme,
+					OCMClient:      fakeOcm,
+					Cache:          cache,
+					SnapshotWriter: snapshotWriter,
+				},
+			}
+
+			t.Log("reconciling configuration")
+			_, err = cr.Reconcile(context.Background(), ctrl.Request{
+				NamespacedName: types.NamespacedName{
+					Namespace: configuration.Namespace,
+					Name:      configuration.Name,
+				},
+			})
+			require.NoError(t, err)
+
+			getErr := client.Get(context.Background(), types.NamespacedName{
+				Namespace: configuration.Namespace,
+				Name:      configuration.Name,
+			}, configuration)
+			require.NoError(t, getErr)
+
+			snapshotOutput := &v1alpha1.Snapshot{}
+			err = client.Get(context.Background(), types.NamespacedName{
+				Namespace: configuration.Namespace,
+				Name:      configuration.Status.SnapshotName,
+			}, snapshotOutput)
+
+			t.Log("verifying that the strategic merge was performed")
+			args := cache.PushDataCallingArgumentsOnCall(0)
+			data := args.Content
+			sourceFile := extractFileFromTarGz(t, io.NopCloser(bytes.NewBuffer([]byte(data))), "merge-target.yaml")
+			deployment := appsv1.Deployment{}
+			err = yaml.Unmarshal(sourceFile, &deployment)
+			assert.NoError(t, err)
+			assert.Equal(t, int32(2), *deployment.Spec.Replicas, "has correct number of replicas")
+			assert.Equal(t, 2, len(deployment.Spec.Template.Spec.Containers), "has correct number of containers")
+			assert.Equal(t, corev1.PullPolicy("Always"), deployment.Spec.Template.Spec.Containers[0].ImagePullPolicy)
+
+			close(recorder.Events)
+			event := ""
+			for e := range recorder.Events {
+				if strings.Contains(e, "Reconciliation finished, next run in") {
+					event = e
+
+					break
+				}
+			}
+			assert.Contains(t, event, "Reconciliation finished, next run in")
+		})
 	}
-	assert.Contains(t, event, "Reconciliation finished, next run in")
 }
 
 func createGitRepository(name, namespace, artifactURL, checksum string) *sourcev1.GitRepository {

diff --git a/controllers/mutation_reconcile_looper.go b/controllers/mutation_reconcile_looper.go
@@ -6,6 +6,7 @@ package controllers
 
 import (
 	"bytes"
+	"compress/gzip"
 	"context"
 	"encoding/json"
 	"errors"
@@ -28,6 +29,7 @@ import (
 	"github.com/mandelsoft/spiff/spiffing"
 	"github.com/mandelsoft/vfs/pkg/osfs"
 	"github.com/open-component-model/ocm-controller/pkg/snapshot"
+	"github.com/open-component-model/ocm-controller/pkg/untar"
 	ocmcore "github.com/open-component-model/ocm/pkg/contexts/ocm"
 	"github.com/open-component-model/ocm/pkg/utils/tarutils"
 	"gopkg.in/yaml.v3"
@@ -1023,8 +1025,19 @@ func (m *MutationReconcileLooper) mutatePatchStrategicMerge(
 			v1alpha1.SourceArtifactChecksumKey: digest,
 		}
 
-		if err := tar.Untar(bytes.NewReader(data), workDir); err != nil {
-			return "", ocmmetav1.Identity{}, fmt.Errorf("failed to untar data from source: %w", err)
+		if _, err := gzip.NewReader(bytes.NewReader(data)); err == nil {
+			if err := tar.Untar(bytes.NewReader(data), workDir); err != nil {
+				return "", ocmmetav1.Identity{}, fmt.Errorf("failed to untar data from source: %w", err)
+			}
+		} else {
+			const perm = 0o755
+			if err := os.MkdirAll(workDir, perm); err != nil {
+				return "", ocmmetav1.Identity{}, fmt.Errorf("failed to create work dir: %w", err)
+			}
+
+			if err := untar.Untar(bytes.NewReader(data), workDir); err != nil {
+				return "", ocmmetav1.Identity{}, fmt.Errorf("failed to untar data from source without gzip: %w", err)
+			}
 		}
 	}
 

diff --git a/controllers/testdata/git-repo.tar b/controllers/testdata/git-repo.tar
diff --git a/pkg/status/identifiable_contract.go b/pkg/status/identifiable_contract.go
@@ -1,3 +1,7 @@
+// SPDX-FileCopyrightText: 2022 SAP SE or an SAP affiliate company and Open Component Model contributors.
+//
+// SPDX-License-Identifier: Apache-2.0
+
 package status
 
 import (

diff --git a/pkg/status/mutate_condition_status.go b/pkg/status/mutate_condition_status.go
@@ -1,3 +1,7 @@
+// SPDX-FileCopyrightText: 2022 SAP SE or an SAP affiliate company and Open Component Model contributors.
+//
+// SPDX-License-Identifier: Apache-2.0
+
 package status
 
 import (