Skip to content

Commit

Permalink
Don't manage ACCESS_TOKEN_ variables for unmanaged cluster git repos (
Browse files Browse the repository at this point in the history 
#303)
  • Loading branch information
@bastjan
bastjan authored Aug 2, 2024
1 parent 187a317 commit 5e1c8e5
Show file tree
Hide file tree
Showing 2 changed files with 121 additions and 19 deletions.
9 changes: 5 additions & 4 deletions controllers/cluster_compile_pipeline_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,11 @@ func (r *ClusterCompilePipelineReconciler) Reconcile(ctx context.Context, reques
}
}

if ensureClusterCiVariable(tenant, instance) {
err = r.Client.Update(ctx, tenant)
if err != nil {
return reconcile.Result{}, err
if instance.GetGitTemplate().RepoType != synv1alpha1.UnmanagedRepoType {
if ensureClusterCiVariable(tenant, instance) {
if err := r.Client.Update(ctx, tenant); err != nil {
return reconcile.Result{}, err
}
}
}

Expand Down
131 changes: 116 additions & 15 deletions controllers/cluster_compile_pipeline_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,22 @@ package controllers_test

import (
"context"
"fmt"
"testing"
"time"

"github.com/stretchr/testify/assert"

synv1alpha1 "github.com/projectsyn/lieutenant-operator/api/v1alpha1"
"github.com/projectsyn/lieutenant-operator/controllers"
"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"

synv1alpha1 "github.com/projectsyn/lieutenant-operator/api/v1alpha1"
"github.com/projectsyn/lieutenant-operator/controllers"
)

func Test_AddClusterToPipelineStatus(t *testing.T) {
func Test_ClusterCompilePipelineReconciler_AddClusterToPipelineStatus(t *testing.T) {
tenant := &synv1alpha1.Tenant{
ObjectMeta: metav1.ObjectMeta{
Name: "t-tenant",
Expand Down Expand Up @@ -50,7 +52,7 @@ func Test_AddClusterToPipelineStatus(t *testing.T) {
assert.Contains(t, mod_tenant.Status.CompilePipeline.Clusters, "c-cluster")
}

func Test_RemoveClusterFromPipelineStatus(t *testing.T) {
func Test_ClusterCompilePipelineReconciler_RemoveClusterFromPipelineStatus(t *testing.T) {
tenant := &synv1alpha1.Tenant{
ObjectMeta: metav1.ObjectMeta{
Name: "t-tenant",
Expand Down Expand Up @@ -90,7 +92,7 @@ func Test_RemoveClusterFromPipelineStatus(t *testing.T) {
assert.NotContains(t, mod_tenant.Status.CompilePipeline.Clusters, "c-cluster")
}

func Test_RemoveClusterFromPipelineStatus_WhenDeleting(t *testing.T) {
func Test_ClusterCompilePipelineReconciler_RemoveClusterFromPipelineStatus_WhenDeleting(t *testing.T) {
now := metav1.NewTime(time.Now())
tenant := &synv1alpha1.Tenant{
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -133,7 +135,7 @@ func Test_RemoveClusterFromPipelineStatus_WhenDeleting(t *testing.T) {
assert.NotContains(t, mod_tenant.Finalizers, synv1alpha1.PipelineFinalizerName)
}

func Test_FinalizerAdded(t *testing.T) {
func Test_ClusterCompilePipelineReconciler_FinalizerAdded(t *testing.T) {
cluster := &synv1alpha1.Cluster{
ObjectMeta: metav1.ObjectMeta{
Name: "c-cluster",
Expand All @@ -160,7 +162,7 @@ func Test_FinalizerAdded(t *testing.T) {
assert.Contains(t, mod_cluster.Finalizers, synv1alpha1.PipelineFinalizerName)
}

func Test_RemoveClusterFromPipelineStatus_EnableUnset(t *testing.T) {
func Test_ClusterCompilePipelineReconciler_RemoveClusterFromPipelineStatus_EnableUnset(t *testing.T) {
tenant := &synv1alpha1.Tenant{
ObjectMeta: metav1.ObjectMeta{
Name: "t-tenant",
Expand Down Expand Up @@ -199,7 +201,7 @@ func Test_RemoveClusterFromPipelineStatus_EnableUnset(t *testing.T) {
assert.NotContains(t, mod_tenant.Status.CompilePipeline.Clusters, "c-cluster")
}

func Test_NoChangeIfClusterInList(t *testing.T) {
func Test_ClusterCompilePipelineReconciler_NoChangeIfClusterInList(t *testing.T) {
tenant := &synv1alpha1.Tenant{
ObjectMeta: metav1.ObjectMeta{
Name: "t-tenant",
Expand Down Expand Up @@ -239,7 +241,7 @@ func Test_NoChangeIfClusterInList(t *testing.T) {
assert.Contains(t, mod_tenant.Status.CompilePipeline.Clusters, "c-cluster")
}

func Test_LeaveOtherListEntriesBe_WhenRemoving(t *testing.T) {
func Test_ClusterCompilePipelineReconciler_LeaveOtherListEntriesBe_WhenRemoving(t *testing.T) {
tenant := &synv1alpha1.Tenant{
ObjectMeta: metav1.ObjectMeta{
Name: "t-tenant",
Expand Down Expand Up @@ -280,7 +282,7 @@ func Test_LeaveOtherListEntriesBe_WhenRemoving(t *testing.T) {
assert.Contains(t, mod_tenant.Status.CompilePipeline.Clusters, "c-other-cluster")
}

func Test_LeaveOtherListEntriesBe_WhenAdding(t *testing.T) {
func Test_ClusterCompilePipelineReconciler_LeaveOtherListEntriesBe_WhenAdding(t *testing.T) {
tenant := &synv1alpha1.Tenant{
ObjectMeta: metav1.ObjectMeta{
Name: "t-tenant",
Expand Down Expand Up @@ -320,7 +322,7 @@ func Test_LeaveOtherListEntriesBe_WhenAdding(t *testing.T) {
assert.Contains(t, mod_tenant.Status.CompilePipeline.Clusters, "c-cluster")
assert.Contains(t, mod_tenant.Status.CompilePipeline.Clusters, "c-other-cluster")
}
func Test_CiVariableNotUpdated_IfNotEnabled(t *testing.T) {
func Test_ClusterCompilePipelineReconciler_CiVariableNotUpdated_IfNotEnabled(t *testing.T) {
tenant := &synv1alpha1.Tenant{
ObjectMeta: metav1.ObjectMeta{
Name: "t-tenant",
Expand Down Expand Up @@ -364,7 +366,7 @@ func Test_CiVariableNotUpdated_IfNotEnabled(t *testing.T) {
assert.Empty(t, mod_tenant.GetGitTemplate().CIVariables)
}

func Test_CiVariableNotUpdated_IfNoToken(t *testing.T) {
func Test_ClusterCompilePipelineReconciler_CiVariableNotUpdated_IfNoToken(t *testing.T) {
tenant := &synv1alpha1.Tenant{
ObjectMeta: metav1.ObjectMeta{
Name: "t-tenant",
Expand Down Expand Up @@ -403,7 +405,7 @@ func Test_CiVariableNotUpdated_IfNoToken(t *testing.T) {
assert.Empty(t, mod_tenant.GetGitTemplate().CIVariables)
}

func Test_CiVariableUpdated_IfEnabled(t *testing.T) {
func Test_ClusterCompilePipelineReconciler_CiVariableUpdated_IfEnabled(t *testing.T) {
tenant := &synv1alpha1.Tenant{
ObjectMeta: metav1.ObjectMeta{
Name: "t-tenant",
Expand Down Expand Up @@ -457,7 +459,8 @@ func Test_CiVariableUpdated_IfEnabled(t *testing.T) {
assert.Equal(t, mod_tenant.Spec.GitRepoTemplate.CIVariables[0].GitlabOptions.Protected, false)
assert.Equal(t, mod_tenant.Spec.GitRepoTemplate.CIVariables[0].GitlabOptions.Raw, true)
}
func Test_KeepListInAlphabeticalOrder(t *testing.T) {

func Test_ClusterCompilePipelineReconciler_KeepListInAlphabeticalOrder(t *testing.T) {
tenant := &synv1alpha1.Tenant{
ObjectMeta: metav1.ObjectMeta{
Name: "t-tenant",
Expand Down Expand Up @@ -553,3 +556,101 @@ func clusterCompilePipelineReconciler(c client.Client) *controllers.ClusterCompi
}
return &r
}

func Test_ClusterCompilePipelineReconciler_CIVariable_IgnoreUnmanagedRepo(t *testing.T) {
for _, tc := range []struct {
preExisting bool
clusterPipelineEnabled bool
}{
{
preExisting: false,
clusterPipelineEnabled: true,
},
{
preExisting: true,
clusterPipelineEnabled: true,
},
{
preExisting: true,
clusterPipelineEnabled: false,
},
} {
t.Run(fmt.Sprintf("PreExisting=%t,ClusterPipelineEnabled=%t", tc.preExisting, tc.clusterPipelineEnabled), func(t *testing.T) {
tenant := &synv1alpha1.Tenant{
ObjectMeta: metav1.ObjectMeta{
Name: "t-tenant",
Namespace: "lieutenant",
},
Spec: synv1alpha1.TenantSpec{
CompilePipeline: &synv1alpha1.CompilePipelineSpec{
Enabled: true,
},
GitRepoTemplate: &synv1alpha1.GitRepoTemplate{},
},
Status: synv1alpha1.TenantStatus{
CompilePipeline: &synv1alpha1.CompilePipelineStatus{
Clusters: []string{"c-cluster"},
},
},
}
cluster := &synv1alpha1.Cluster{
ObjectMeta: metav1.ObjectMeta{
Name: "c-cluster",
Namespace: "lieutenant",
Finalizers: []string{synv1alpha1.PipelineFinalizerName},
},
Spec: synv1alpha1.ClusterSpec{
TenantRef: corev1.LocalObjectReference{
Name: "t-tenant",
},
EnableCompilePipeline: tc.clusterPipelineEnabled,
GitRepoTemplate: &synv1alpha1.GitRepoTemplate{
RepoType: synv1alpha1.UnmanagedRepoType,
AccessToken: synv1alpha1.AccessToken{
SecretRef: "my-secret",
},
},
},
}
preExistingVar := synv1alpha1.EnvVar{
Name: "ACCESS_TOKEN_c_cluster",
Value: "whatever",
}
if tc.preExisting {
tenant.Spec.GitRepoTemplate.CIVariables = []synv1alpha1.EnvVar{preExistingVar}
}

c := preparePipelineTestClient(t, tenant, cluster)
r := clusterCompilePipelineReconciler(c)
ctx := context.Background()

_, err := r.Reconcile(ctx, requestFor(cluster))
assert.NoError(t, err)

modTenant := &synv1alpha1.Tenant{}
err = c.Get(ctx, types.NamespacedName{Name: "t-tenant", Namespace: "lieutenant"}, modTenant)
assert.NoError(t, err)

findCIVar := func(name string) (ciVar synv1alpha1.EnvVar, ok bool) {
for _, ciVar := range modTenant.GetGitTemplate().CIVariables {
if ciVar.Name == name {
return ciVar, true
}
}
return synv1alpha1.EnvVar{}, false
}

if tc.clusterPipelineEnabled && tc.preExisting {
ciVar, ok := findCIVar("ACCESS_TOKEN_c_cluster")
require.True(t, ok)
assert.Equal(t, preExistingVar, ciVar, "Unmanged CIVariable should not have been changed")
} else if tc.clusterPipelineEnabled && !tc.preExisting {
_, ok := findCIVar("ACCESS_TOKEN_c_cluster")
assert.False(t, ok, "Unmanged CIVariable should not have been added")
} else if !tc.clusterPipelineEnabled && tc.preExisting {
_, ok := findCIVar("ACCESS_TOKEN_c_cluster")
assert.True(t, ok, "Unmanged CIVariable should not have been removed")
}
})
}
}

0 comments on commit 5e1c8e5

Please sign in to comment.