fix naming, branch checkout logic and uncompression

Author: Skarlso

apis/delivery/v1alpha1/sync_types.go

@@ -35,7 +35,10 @@ type SyncSpec struct {
 	Prune          bool                    `json:"prune,omitempty"`
 
 	//+optional
-	Branch string `json:"branch,omitempty"`
+	TargetBranch string `json:"targetBranch,omitempty"`
+	//+optional
+	//+kubebuilder:default:=main
+	BaseBranch string `json:"baseBranch,omitempty"`
 	//+optional
 	AutomaticPullRequestCreation bool `json:"automaticPullRequestCreation,omitempty"`
 	//+optional

config/crd/bases/delivery.ocm.software_syncs.yaml

@@ -37,7 +37,8 @@ spec:
             properties:
               automaticPullRequestCreation:
                 type: boolean
-              branch:
+              baseBranch:
+                default: main
                 type: string
               commitTemplate:
                 description: CommitTemplate defines the details of the commit to the
@@ -91,6 +92,8 @@ spec:
                 x-kubernetes-map-type: atomic
               subPath:
                 type: string
+              targetBranch:
+                type: string
             required:
             - commitTemplate
             - interval

controllers/delivery/sync_controller.go

@@ -20,8 +20,10 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/builder"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/log"
+	"sigs.k8s.io/controller-runtime/pkg/predicate"
 
 	ocmv1 "github.com/open-component-model/ocm-controller/api/v1alpha1"
 
@@ -56,7 +58,7 @@ func (r *SyncReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.
 	)
 
 	log := log.FromContext(ctx)
-	log.V(4).Info("starting reconcile loop for snapshot")
+
 	obj := &v1alpha1.Sync{}
 	if err := r.Get(ctx, req.NamespacedName, obj); err != nil {
 		if apierrors.IsNotFound(err) {
@@ -142,6 +144,8 @@ func (r *SyncReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.
 		return ctrl.Result{}, retErr
 	}
 
+	log.V(4).Info("found target snapshot")
+
 	repository := &mpasv1alpha1.Repository{}
 	if err := r.Get(ctx, types.NamespacedName{
 		Namespace: obj.Namespace,
@@ -153,6 +157,8 @@ func (r *SyncReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.
 		return ctrl.Result{}, retErr
 	}
 
+	log.V(4).Info("found target repository")
+
 	authSecret := &corev1.Secret{}
 	if err := r.Get(ctx, types.NamespacedName{
 		Namespace: obj.Namespace,
@@ -164,26 +170,35 @@ func (r *SyncReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.
 		return ctrl.Result{}, retErr
 	}
 
-	branch := obj.Spec.Branch
-	if branch == "" && obj.Spec.AutomaticPullRequestCreation {
-		branch = fmt.Sprintf("branch-%d", time.Now().Unix())
-	} else if branch == "" && !obj.Spec.AutomaticPullRequestCreation {
+	log.V(4).Info("found authentication secret")
+
+	baseBranch := obj.Spec.BaseBranch
+	if baseBranch == "" {
+		baseBranch = "main"
+	}
+	targetBranch := obj.Spec.TargetBranch
+	if targetBranch == "" && obj.Spec.AutomaticPullRequestCreation {
+		targetBranch = fmt.Sprintf("branch-%d", time.Now().Unix())
+	} else if targetBranch == "" && !obj.Spec.AutomaticPullRequestCreation {
 		retErr = fmt.Errorf("branch cannot be empty if automatic pull request creation is not enabled")
 		conditions.MarkFalse(obj, meta.ReadyCondition, v1alpha1.GitRepositoryPushFailedReason, retErr.Error())
 
 		return ctrl.Result{}, retErr
 	}
 
+	log.Info("preparing to push snapshot content", "base", baseBranch, "target", targetBranch)
+
 	// trim any trailing `/` and then just add.
 	log.V(4).Info("crafting artifact URL to download from", "url", snapshot.Status.RepositoryURL)
 	opts := &pkg.PushOptions{
-		URL:      repository.GetRepositoryURL(),
-		Message:  obj.Spec.CommitTemplate.Message,
-		Name:     obj.Spec.CommitTemplate.Name,
-		Email:    obj.Spec.CommitTemplate.Email,
-		Snapshot: snapshot,
-		Branch:   branch,
-		SubPath:  obj.Spec.SubPath,
+		URL:          repository.GetRepositoryURL(),
+		Message:      obj.Spec.CommitTemplate.Message,
+		Name:         obj.Spec.CommitTemplate.Name,
+		Email:        obj.Spec.CommitTemplate.Email,
+		Snapshot:     snapshot,
+		BaseBranch:   baseBranch,
+		TargetBranch: targetBranch,
+		SubPath:      obj.Spec.SubPath,
 	}
 
 	r.parseAuthSecret(authSecret, opts)
@@ -196,10 +211,14 @@ func (r *SyncReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.
 		return ctrl.Result{}, retErr
 	}
 
+	log.Info("target content pushed with digest", "base", baseBranch, "target", targetBranch, "digest", digest)
+
 	obj.Status.Digest = digest
 
 	if obj.Spec.AutomaticPullRequestCreation {
-		if err := r.Provider.CreatePullRequest(ctx, branch, *obj, *repository); err != nil {
+		log.Info("automatic pull-request creation is enabled, preparing to create a pull request")
+
+		if err := r.Provider.CreatePullRequest(ctx, targetBranch, *obj, *repository); err != nil {
 			retErr = fmt.Errorf("failed to create pull request: %w", err)
 			conditions.MarkFalse(obj, meta.ReadyCondition, v1alpha1.CreatePullRequestFailedReason, retErr.Error())
 
@@ -212,13 +231,15 @@ func (r *SyncReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.
 	// block at the very end.
 	conditions.Delete(obj, meta.ReadyCondition)
 
+	log.Info("successfully reconciled sync object")
+
 	return result, retErr
 }
 
 // SetupWithManager sets up the controller with the Manager.
 func (r *SyncReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	return ctrl.NewControllerManagedBy(mgr).
-		For(&v1alpha1.Sync{}).
+		For(&v1alpha1.Sync{}, builder.WithPredicates(predicate.GenerationChangedPredicate{})).
 		Complete(r)
 }
 

controllers/delivery/sync_controller_test.go

@@ -69,7 +69,7 @@ func TestSyncReconciler(t *testing.T) {
 			RepositoryRef: v1.LocalObjectReference{
 				Name: repository.Name,
 			},
-			Branch: "main",
+			TargetBranch: "main",
 			CommitTemplate: &v1alpha1.CommitTemplate{
 				Name:    "Skarlso",
 				Email:   "[email protected]",
@@ -151,7 +151,7 @@ func TestSyncReconcilerWithAutomaticPullRequest(t *testing.T) {
 			RepositoryRef: v1.LocalObjectReference{
 				Name: repository.Name,
 			},
-			Branch: "main",
+			TargetBranch: "main",
 			CommitTemplate: &v1alpha1.CommitTemplate{
 				Name:    "Skarlso",
 				Email:   "[email protected]",

go.mod

@@ -5,6 +5,7 @@ go 1.20
 require (
 	code.gitea.io/sdk/gitea v0.15.1
 	github.com/Masterminds/semver/v3 v3.2.0
+	github.com/containers/image/v5 v5.21.1
 	github.com/fluxcd/go-git-providers v0.15.0
 	github.com/fluxcd/pkg/apis/meta v0.19.0
 	github.com/fluxcd/pkg/runtime v0.27.0
@@ -48,7 +49,6 @@ require (
 	github.com/cloudflare/circl v1.3.2 // indirect
 	github.com/containerd/containerd v1.6.18 // indirect
 	github.com/containerd/stargz-snapshotter/estargz v0.14.1 // indirect
-	github.com/containers/image/v5 v5.21.1 // indirect
 	github.com/containers/libtrust v0.0.0-20200511145503-9c3a6c22cd9a // indirect
 	github.com/containers/ocicrypt v1.1.5 // indirect
 	github.com/containers/storage v1.42.0 // indirect

pkg/git.go

@@ -32,15 +32,16 @@ type Auth struct {
 
 // PushOptions contains settings for a push action.
 type PushOptions struct {
-	Auth     *Auth
-	URL      string
-	Message  string
-	Name     string
-	Email    string
-	Snapshot *ocmv1.Snapshot
-	Branch   string
-	SubPath  string
-	Prune    bool
+	Auth         *Auth
+	URL          string
+	Message      string
+	Name         string
+	Email        string
+	Snapshot     *ocmv1.Snapshot
+	BaseBranch   string
+	TargetBranch string
+	SubPath      string
+	Prune        bool
 }
 
 // Git defines an interface to abstract git operations.

pkg/gogit/git.go

@@ -11,6 +11,7 @@ import (
 	"path/filepath"
 	"time"
 
+	"github.com/containers/image/v5/pkg/compression"
 	"github.com/go-git/go-git/v5"
 	"github.com/go-git/go-git/v5/plumbing"
 	"github.com/go-git/go-git/v5/plumbing/object"
@@ -65,7 +66,7 @@ func (g *Git) Push(ctx context.Context, opts *pkg.PushOptions) (string, error) {
 	cloneOptions := &git.CloneOptions{
 		URL:           opts.URL,
 		Depth:         1,
-		ReferenceName: plumbing.ReferenceName(fmt.Sprintf("refs/heads/%s", opts.Branch)),
+		ReferenceName: plumbing.ReferenceName(fmt.Sprintf("refs/heads/%s", opts.BaseBranch)),
 		Auth:          auth,
 	}
 
@@ -79,6 +80,15 @@ func (g *Git) Push(ctx context.Context, opts *pkg.PushOptions) (string, error) {
 		return "", fmt.Errorf("failed to create a worktree: %w", err)
 	}
 
+	if opts.TargetBranch != opts.BaseBranch {
+		if err := w.Checkout(&git.CheckoutOptions{
+			Branch: plumbing.NewBranchReferenceName(opts.TargetBranch),
+			Create: true,
+		}); err != nil {
+			return "", fmt.Errorf("failed to checkout new branch: %w", err)
+		}
+	}
+
 	dir = filepath.Join(dir, opts.SubPath)
 	if err := os.MkdirAll(dir, 0777); err != nil {
 		return "", fmt.Errorf("failed to create subPath: %w", err)
@@ -94,9 +104,15 @@ func (g *Git) Push(ctx context.Context, opts *pkg.PushOptions) (string, error) {
 		return "", fmt.Errorf("failed to fetch blob for digest: %w", err)
 	}
 
+	uncompressed, _, err := compression.AutoDecompress(blob)
+	if err != nil {
+		return "", fmt.Errorf("failed to auto decompress: %w", err)
+	}
+	defer uncompressed.Close()
+
 	// we only care about the error if it is NOT a header error. Otherwise, we assume the content
 	// wasn't compressed.
-	if err = Untar(blob, dir); err != nil {
+	if err = Untar(uncompressed, dir); err != nil {
 		return "", fmt.Errorf("failed to untar content: %w", err)
 	}
 
@@ -112,6 +128,7 @@ func (g *Git) Push(ctx context.Context, opts *pkg.PushOptions) (string, error) {
 			When:  time.Now(),
 		},
 	}
+
 	commit, err := w.Commit("Uploading snapshot to location", commitOpts)
 	if err != nil {
 		return "", fmt.Errorf("failed to commit changes: %w", err)