Skip to content

Commit

Permalink
Run all tests, fix panics
Browse files Browse the repository at this point in the history
  • Loading branch information
@Richard87
Richard87 committed Dec 13, 2023
1 parent 9d69ce7 commit 497e683
Show file tree
Hide file tree
Showing 6 changed files with 133 additions and 42 deletions.
10 changes: 5 additions & 5 deletions pkg/pipeline/prepare_pipelines.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ func (ctx *pipelineContext) buildTasks(envName string, tasks []pipelinev1.Task,
task.ObjectMeta.Labels[k] = v
}

if val, ok := task.ObjectMeta.Labels[labels.AzureWorkloadIdenityUse]; ok && val == "true" {
if val, ok := task.ObjectMeta.Labels[labels.AzureWorkloadIdentityUse]; ok && val == "true" {
validateAzureSkipContainers(&task)
}

Expand All @@ -263,7 +263,7 @@ func (ctx *pipelineContext) buildTasks(envName string, tasks []pipelinev1.Task,
}

func validateAzureSkipContainers(task *pipelinev1.Task) {
skip := strings.Split(task.ObjectMeta.Annotations[annotations.AzureWorkloadIdentiySkipContainers], ";")
skip := strings.Split(task.ObjectMeta.Annotations[annotations.AzureWorkloadIdentitySkipContainers], ";")
var updatedSkipNames []string

for _, containerName := range skip {
Expand Down Expand Up @@ -291,13 +291,13 @@ func validateAzureSkipContainers(task *pipelinev1.Task) {
tektonInitContainers := []string{"place-scripts", "prepare"}

for _, tekton := range tektonInitContainers {
if !slices.Contains(skip, tekton) {
if !slices.Contains(updatedSkipNames, tekton) {
log.Infof("Ignoring '%s' container for azure workload identity in task '%s'", tekton, task.Name)
skip = append(skip, tekton)
updatedSkipNames = append(updatedSkipNames, tekton)
}
}

task.ObjectMeta.Annotations[annotations.AzureWorkloadIdentiySkipContainers] = strings.Join(updatedSkipNames, ";")
task.ObjectMeta.Annotations[annotations.AzureWorkloadIdentitySkipContainers] = strings.Join(updatedSkipNames, ";")
}

func ensureCorrectSecureContext(task *pipelinev1.Task) {
Expand Down
110 changes: 78 additions & 32 deletions pkg/pipeline/prepare_pipelines_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package pipeline

import (
"context"
"strings"
"testing"

commonUtils "github.com/equinor/radix-common/utils"
Expand All @@ -10,8 +11,10 @@ import (
"github.com/equinor/radix-operator/pkg/apis/utils"
radixclientfake "github.com/equinor/radix-operator/pkg/client/clientset/versioned/fake"
"github.com/equinor/radix-tekton/pkg/models/env"
"github.com/equinor/radix-tekton/pkg/utils/labels"
"github.com/golang/mock/gomock"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
pipelinev1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
tektonclientfake "github.com/tektoncd/pipeline/pkg/client/clientset/versioned/fake"
corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -194,7 +197,7 @@ func Test_pipelineContext_createPipeline(t *testing.T) {
fields fields
args args
wantErr func(t *testing.T, err error)
assertScenario func(t *testing.T, ctx *pipelineContext)
assertScenario func(t *testing.T, ctx *pipelineContext, pipelineName string)
}{
{
name: "one default task",
Expand All @@ -215,19 +218,19 @@ func Test_pipelineContext_createPipeline(t *testing.T) {
wantErr: func(t *testing.T, err error) {
assert.Nil(t, err)
},
assertScenario: func(t *testing.T, ctx *pipelineContext) {
pipeline, err := ctx.tektonClient.TektonV1().Pipelines(ctx.env.GetAppNamespace()).Get(context.Background(), "pipeline1", metav1.GetOptions{})
assert.Nil(t, err)
assert.Equal(t, 1, len(pipeline.Spec.Tasks))
assertScenario: func(t *testing.T, ctx *pipelineContext, pipelineName string) {
pipeline, err := ctx.tektonClient.TektonV1().Pipelines(ctx.env.GetAppNamespace()).Get(context.Background(), pipelineName, metav1.GetOptions{})
require.NoError(t, err)
require.Len(t, pipeline.Spec.Tasks, 1)
task := pipeline.Spec.Tasks[0]
assert.Equal(t, "task1", task.Name)
assert.Equal(t, 1, len(task.TaskSpec.Steps))
require.Len(t, task.TaskSpec.Steps, 1)
assert.Equal(t, "step1", task.TaskSpec.Steps[0].Name)
assert.Equal(t, 1, len(task.TaskSpec.Sidecars))
assert.Equal(t, "sidecar1", task.TaskSpec.Sidecars[0].Name)
assert.Equal(t, "image1", task.TaskSpec.StepTemplate.Image)
assert.NotNilf(t, pipeline.ObjectMeta.OwnerReferences, "Expected owner reference to be set")
assert.Len(t, pipeline.ObjectMeta.OwnerReferences, 1)
require.Len(t, pipeline.ObjectMeta.OwnerReferences, 1)
assert.Equal(t, "RadixApplication", pipeline.ObjectMeta.OwnerReferences[0].Kind)
assert.Equal(t, appName, pipeline.ObjectMeta.OwnerReferences[0].Name)
},
Expand Down Expand Up @@ -262,12 +265,12 @@ func Test_pipelineContext_createPipeline(t *testing.T) {
wantErr: func(t *testing.T, err error) {
assert.Nil(t, err)
},
assertScenario: func(t *testing.T, ctx *pipelineContext) {
pipeline, err := ctx.tektonClient.TektonV1().Pipelines(ctx.env.GetAppNamespace()).Get(context.Background(), "pipeline1", metav1.GetOptions{})
assert.Nil(t, err)
assert.Equal(t, 1, len(pipeline.Spec.Tasks))
assertScenario: func(t *testing.T, ctx *pipelineContext, pipelineName string) {
pipeline, err := ctx.tektonClient.TektonV1().Pipelines(ctx.env.GetAppNamespace()).Get(context.Background(), pipelineName, metav1.GetOptions{})
require.NoError(t, err)
require.Equal(t, 1, len(pipeline.Spec.Tasks))
task := pipeline.Spec.Tasks[0]
assert.Equal(t, 1, len(task.TaskSpec.Steps))
require.Equal(t, 1, len(task.TaskSpec.Steps))
step := task.TaskSpec.Steps[0]
assert.NotNil(t, step.SecurityContext)
assert.Equal(t, commonUtils.BoolPtr(true), step.SecurityContext.RunAsNonRoot)
Expand Down Expand Up @@ -312,12 +315,12 @@ func Test_pipelineContext_createPipeline(t *testing.T) {
wantErr: func(t *testing.T, err error) {
assert.Nil(t, err)
},
assertScenario: func(t *testing.T, ctx *pipelineContext) {
pipeline, err := ctx.tektonClient.TektonV1().Pipelines(ctx.env.GetAppNamespace()).Get(context.Background(), "pipeline1", metav1.GetOptions{})
assert.Nil(t, err)
assert.Equal(t, 1, len(pipeline.Spec.Tasks))
assertScenario: func(t *testing.T, ctx *pipelineContext, pipelineName string) {
pipeline, err := ctx.tektonClient.TektonV1().Pipelines(ctx.env.GetAppNamespace()).Get(context.Background(), pipelineName, metav1.GetOptions{})
require.NoError(t, err)
require.Equal(t, 1, len(pipeline.Spec.Tasks))
task := pipeline.Spec.Tasks[0]
assert.Equal(t, 1, len(task.TaskSpec.Sidecars))
require.Equal(t, 1, len(task.TaskSpec.Sidecars))
sidecar := task.TaskSpec.Sidecars[0]
assert.NotNil(t, sidecar.SecurityContext)
assert.Equal(t, commonUtils.BoolPtr(true), sidecar.SecurityContext.RunAsNonRoot)
Expand Down Expand Up @@ -359,10 +362,10 @@ func Test_pipelineContext_createPipeline(t *testing.T) {
wantErr: func(t *testing.T, err error) {
assert.Nil(t, err)
},
assertScenario: func(t *testing.T, ctx *pipelineContext) {
pipeline, err := ctx.tektonClient.TektonV1().Pipelines(ctx.env.GetAppNamespace()).Get(context.Background(), "pipeline1", metav1.GetOptions{})
assert.Nil(t, err)
assert.Equal(t, 1, len(pipeline.Spec.Tasks))
assertScenario: func(t *testing.T, ctx *pipelineContext, pipelineName string) {
pipeline, err := ctx.tektonClient.TektonV1().Pipelines(ctx.env.GetAppNamespace()).Get(context.Background(), pipelineName, metav1.GetOptions{})
require.NoError(t, err)
require.Equal(t, 1, len(pipeline.Spec.Tasks))
task := pipeline.Spec.Tasks[0]
stepTemplate := task.TaskSpec.StepTemplate
assert.NotNil(t, stepTemplate)
Expand Down Expand Up @@ -400,9 +403,11 @@ func Test_pipelineContext_createPipeline(t *testing.T) {
wantErr: func(t *testing.T, err error) {
assert.Nil(t, err)
},
assertScenario: func(t *testing.T, ctx *pipelineContext) {
pipeline, err := ctx.tektonClient.TektonV1().Pipelines(ctx.env.GetAppNamespace()).Get(context.Background(), "pipeline1", metav1.GetOptions{})
assert.Nil(t, err)
assertScenario: func(t *testing.T, ctx *pipelineContext, pipelineName string) {
pipeline, err := ctx.tektonClient.TektonV1().Pipelines(ctx.env.GetAppNamespace()).Get(context.Background(), pipelineName, metav1.GetOptions{})
require.NoError(t, err)
require.Len(t, pipeline.Spec.Tasks, 1)
require.Len(t, pipeline.Spec.Tasks[0].TaskSpec.Steps, 1)
step := pipeline.Spec.Tasks[0].TaskSpec.Steps[0]
assert.NotNil(t, step.SecurityContext.RunAsUser)
assert.Equal(t, 10, *step.SecurityContext.RunAsUser)
Expand Down Expand Up @@ -434,9 +439,11 @@ func Test_pipelineContext_createPipeline(t *testing.T) {
wantErr: func(t *testing.T, err error) {
assert.Nil(t, err)
},
assertScenario: func(t *testing.T, ctx *pipelineContext) {
pipeline, err := ctx.tektonClient.TektonV1().Pipelines(ctx.env.GetAppNamespace()).Get(context.Background(), "pipeline1", metav1.GetOptions{})
assert.Nil(t, err)
assertScenario: func(t *testing.T, ctx *pipelineContext, pipelineName string) {
pipeline, err := ctx.tektonClient.TektonV1().Pipelines(ctx.env.GetAppNamespace()).Get(context.Background(), pipelineName, metav1.GetOptions{})
require.NoError(t, err)
require.Len(t, pipeline.Spec.Tasks, 1)
require.Len(t, pipeline.Spec.Tasks[0].TaskSpec.Sidecars, 1)
sidecar := pipeline.Spec.Tasks[0].TaskSpec.Sidecars[0]
assert.NotNil(t, sidecar.SecurityContext.RunAsUser)
assert.Equal(t, 10, *sidecar.SecurityContext.RunAsUser)
Expand Down Expand Up @@ -465,17 +472,54 @@ func Test_pipelineContext_createPipeline(t *testing.T) {
wantErr: func(t *testing.T, err error) {
assert.Nil(t, err)
},
assertScenario: func(t *testing.T, ctx *pipelineContext) {
pipeline, err := ctx.tektonClient.TektonV1().Pipelines(ctx.env.GetAppNamespace()).Get(context.Background(), "pipeline1", metav1.GetOptions{})
assert.Nil(t, err)
assertScenario: func(t *testing.T, ctx *pipelineContext, pipelineName string) {
pipeline, err := ctx.tektonClient.TektonV1().Pipelines(ctx.env.GetAppNamespace()).Get(context.Background(), pipelineName, metav1.GetOptions{})
require.NoError(t, err)
require.Len(t, pipeline.Spec.Tasks, 1)
stepTemplate := pipeline.Spec.Tasks[0].TaskSpec.StepTemplate
assert.NotNil(t, stepTemplate.SecurityContext.RunAsUser)
require.NotNil(t, stepTemplate.SecurityContext.RunAsUser)
assert.Equal(t, 10, *stepTemplate.SecurityContext.RunAsUser)
assert.Nil(t, stepTemplate.SecurityContext.RunAsGroup)
assert.NotNil(t, stepTemplate.SecurityContext.RunAsGroup)
assert.Equal(t, 20, stepTemplate.SecurityContext.RunAsGroup)
},
},
{
name: "Test validateAzureSkipContainers in task stepTemplate",
args: args{
envName: envDev,
pipeline: getTestPipeline(func(pipeline *pipelinev1.Pipeline) {
pipeline.ObjectMeta.Name = "pipeline1"
pipeline.Spec.Tasks = append(pipeline.Spec.Tasks, pipelinev1.PipelineTask{Name: "identity", TaskRef: &pipelinev1.TaskRef{Name: "task1"}})
}),
tasks: []pipelinev1.Task{{
ObjectMeta: metav1.ObjectMeta{
Name: "task1",
Annotations: map[string]string{"azure.workload.identity/skip-containers": "skip-id"},
Labels: map[string]string{labels.AzureWorkloadIdentityUse: "true"},
},
Spec: pipelinev1.TaskSpec{Steps: []pipelinev1.Step{
{Name: "get-secret"}, {Name: "skip-id"}},
}},
},
},
wantErr: func(t *testing.T, err error) {
assert.Nil(t, err)
},
assertScenario: func(t *testing.T, ctx *pipelineContext, pipelineName string) {
pipeline, err := ctx.tektonClient.TektonV1().Pipelines(ctx.env.GetAppNamespace()).Get(context.Background(), pipelineName, metav1.GetOptions{})
require.NoError(t, err)
require.Len(t, pipeline.Spec.Tasks, 1)
task, err := ctx.tektonClient.TektonV1().Tasks(ctx.env.GetAppNamespace()).Get(context.Background(), pipeline.Spec.Tasks[0].TaskRef.Name, metav1.GetOptions{})
require.NoError(t, err)

skipContainers := strings.Split(task.Annotations["azure.workload.identity/skip-containers"], ";")
assert.Len(t, skipContainers, 3)
assert.Contains(t, skipContainers, "step-skip-id")
assert.Contains(t, skipContainers, "place-scripts")
assert.Contains(t, skipContainers, "prepare")
},
},
}
for _, scenario := range scenarios {
t.Run(scenario.name, func(t *testing.T) {
Expand Down Expand Up @@ -511,7 +555,9 @@ func Test_pipelineContext_createPipeline(t *testing.T) {
if ctx.hash == "" {
ctx.hash = hash
}
scenario.wantErr(t, ctx.createPipeline(scenario.args.envName, scenario.args.pipeline, scenario.args.tasks, scenario.args.timestamp))
err := ctx.createPipeline(scenario.args.envName, scenario.args.pipeline, scenario.args.tasks, scenario.args.timestamp)
scenario.wantErr(t, err)
scenario.assertScenario(t, ctx, scenario.args.pipeline.ObjectMeta.Name)
})
}
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/pipeline/validation/task_validations.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ import (
)

var (
allowedUserAnnotations = []string{annotations.AzureWorkloadIdentiySkipContainers}
allowedUserLabels = []string{labels.AzureWorkloadIdenityUse}
allowedUserAnnotations = []string{annotations.AzureWorkloadIdentitySkipContainers}
allowedUserLabels = []string{labels.AzureWorkloadIdentityUse}
)

// ValidateTask Validate task
Expand Down
47 changes: 46 additions & 1 deletion pkg/pipeline/validation/task_validations_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,10 +105,53 @@ func TestValidateTask(t *testing.T) {
},
expecedErrors: []error{validation.ErrHostPathNotAllowed},
},
{
name: "Test allowed Pipeline labels and annotatoins",
task: pipelinev1.Task{
ObjectMeta: v1.ObjectMeta{
Name: "Test Task",
Labels: map[string]string{
"azure.workload.identity/use": "true",
},
Annotations: map[string]string{
"azure.workload.identity/skip-containers": "skip-id",
},
},
Spec: pipelinev1.TaskSpec{
Steps: []pipelinev1.Step{{}},
},
},
expecedErrors: []error{},
},
{
name: "Test illegal Pipeline labels and annotatoins",
task: pipelinev1.Task{
ObjectMeta: v1.ObjectMeta{
Name: "Test Task",
Labels: map[string]string{
"ILLEGAL_LABEL": "true",
},
Annotations: map[string]string{
"ILLEGAL_ANNOTATION": "true",
},
},
Spec: pipelinev1.TaskSpec{
Steps: []pipelinev1.Step{{}},
},
},
expecedErrors: []error{validation.ErrIllegalTaskLabel, validation.ErrIllegalTaskAnnotation},
},
{
name: "collection of errors",
task: pipelinev1.Task{
ObjectMeta: v1.ObjectMeta{Name: "Test Task"},
ObjectMeta: v1.ObjectMeta{
Name: "Test Task",
Labels: map[string]string{
"ILLEGAL_LABEL": "true",
},
Annotations: map[string]string{
"ILLEGAL_ANNOTATION": "true",
}},
Spec: pipelinev1.TaskSpec{
Volumes: []corev1.Volume{
{
Expand Down Expand Up @@ -138,6 +181,8 @@ func TestValidateTask(t *testing.T) {
validation.ErrHostPathNotAllowed,
validation.ErrRadixVolumeNameNotAllowed,
validation.ErrSecretReferenceNotAllowed,
validation.ErrIllegalTaskAnnotation,
validation.ErrIllegalTaskLabel,
},
},
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/utils/annotations/annotations.go
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
package annotations

const (
AzureWorkloadIdentiySkipContainers = "azure.workload.identity/skip-containers"
AzureWorkloadIdentitySkipContainers = "azure.workload.identity/skip-containers"
)
2 changes: 1 addition & 1 deletion pkg/utils/labels/labels.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import (
)

const (
AzureWorkloadIdenityUse = "azure.workload.identity/use"
AzureWorkloadIdentityUse = "azure.workload.identity/use"
)

// GetLabelsForEnvironment Get Pipeline object labels for a target build environment
Expand Down

0 comments on commit 497e683

Please sign in to comment.