Skip to content

Commit

Permalink
Merge pull request #165 from syntasso/clearer-pipeline-visibility
Browse files Browse the repository at this point in the history
Clearer pipeline visibility
  • Loading branch information
SaphMB authored Jun 14, 2024
2 parents 1a60455 + 1e60178 commit 097a721
Show file tree
Hide file tree
Showing 4 changed files with 189 additions and 21 deletions.
8 changes: 4 additions & 4 deletions lib/pipeline/args.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,15 @@ type PipelineArgs struct {
names map[string]string
}

func NewPipelineArgs(promiseIdentifier, resourceRequestIdentifier, pName, name, namespace string) PipelineArgs {
func NewPipelineArgs(promiseIdentifier, resourceRequestIdentifier, pName, objectName, namespace string) PipelineArgs {
pipelineID := promiseIdentifier + "-promise-pipeline"
if resourceRequestIdentifier != "" {
pipelineID = promiseIdentifier + "-resource-pipeline"
}

names := map[string]string{
"configure-pipeline-name": pipelineName("configure", promiseIdentifier),
"delete-pipeline-name": pipelineName("delete", promiseIdentifier),
"configure-pipeline-name": pipelineName(promiseIdentifier, resourceRequestIdentifier, objectName, pName),
"delete-pipeline-name": pipelineName(promiseIdentifier, resourceRequestIdentifier, objectName, pName),
"promise-id": promiseIdentifier,
"service-account": pipelineID,
"role": pipelineID,
Expand All @@ -21,7 +21,7 @@ func NewPipelineArgs(promiseIdentifier, resourceRequestIdentifier, pName, name,
"resource-request-id": resourceRequestIdentifier,
"namespace": namespace,
"pipeline-name": pName,
"name": name,
"name": objectName,
}

return PipelineArgs{
Expand Down
128 changes: 123 additions & 5 deletions lib/pipeline/configure_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,12 @@ import (
"github.com/go-logr/logr"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
. "github.com/onsi/gomega/gstruct"
"github.com/onsi/gomega/types"

"github.com/syntasso/kratix/api/v1alpha1"
"github.com/syntasso/kratix/lib/pipeline"
batchv1 "k8s.io/api/batch/v1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
Expand All @@ -19,6 +23,9 @@ var _ = Describe("Configure Pipeline", func() {
p v1alpha1.Pipeline
pipelineResources pipeline.PipelineArgs
logger logr.Logger
job *batchv1.Job
err error
labelsMatcher types.GomegaMatcher
)

BeforeEach(func() {
Expand Down Expand Up @@ -48,17 +55,128 @@ var _ = Describe("Configure Pipeline", func() {
}
logger = logr.Logger{}

pipelineResources = pipeline.NewPipelineArgs("test-promise", "test-resource-request", "configure-step", "test-name", "test-namespace")
pipelineResources = pipeline.NewPipelineArgs("test-promise", "", "configure-step", "test-name", "test-namespace")
})

Describe("Pipeline Request Hash", func() {
Describe("Promise Configure Pipeline", func() {
const expectedHash = "9bb58f26192e4ba00f01e2e7b136bbd8"
BeforeEach(func() {
job, err = pipeline.ConfigurePipeline(rr, expectedHash, p, pipelineResources, "test-promise", false, logger)
Expect(err).NotTo(HaveOccurred())

labelsMatcher = MatchAllKeys(Keys{
"kratix.io/hash": Equal(expectedHash),
"kratix-workflow-action": Equal("configure"),
"kratix-workflow-pipeline-name": Equal("configure-step"),
"kratix.io/pipeline-name": Equal("configure-step"),
"kratix-workflow-type": Equal("promise"),
"kratix-workflow-kind": Equal("pipeline.platform.kratix.io"),
"kratix-workflow-promise-version": Equal("v1alpha1"),
"kratix.io/work-type": Equal("promise"),
"kratix.io/promise-name": Equal("test-promise"),
})
})

It("creates a job with the expected metadata", func() {
Expect(job.ObjectMeta).To(MatchFields(IgnoreExtras, Fields{
"Name": HavePrefix("kratix-test-promise-configure-step-"),
"Namespace": Equal("test-namespace"),
"Labels": labelsMatcher,
}))
})

Context("when the pipeline name would exceed the 63 character limit", func() {
BeforeEach(func() {
promise_identifier := "long-long-long-long-promise"
pipeline_name := "also-very-verbose-pipeline"
pipelineResources = pipeline.NewPipelineArgs(promise_identifier, "", pipeline_name, "test-name", "test-namespace")

job, err = pipeline.ConfigurePipeline(rr, expectedHash, p, pipelineResources, "test-promise", false, logger)

labelsMatcher = MatchAllKeys(Keys{
"kratix.io/hash": Equal(expectedHash),
"kratix-workflow-action": Equal("configure"),
"kratix-workflow-pipeline-name": Equal(pipeline_name),
"kratix.io/pipeline-name": Equal(pipeline_name),
"kratix-workflow-type": Equal("promise"),
"kratix-workflow-kind": Equal("pipeline.platform.kratix.io"),
"kratix-workflow-promise-version": Equal("v1alpha1"),
"kratix.io/work-type": Equal("promise"),
"kratix.io/promise-name": Equal(promise_identifier),
})
})

It("truncates the pipeline name to ensure it fits the 63 character limit", func() {
Expect(job.ObjectMeta.Name).To(HaveLen(62))
Expect(job.ObjectMeta).To(MatchFields(IgnoreExtras, Fields{
"Name": HavePrefix("kratix-long-long-long-long-promise-also-very-verbose-pip-"),
"Namespace": Equal("test-namespace"),
"Labels": labelsMatcher,
}))
})
})
})

It("is included as a label to the pipeline job", func() {
job, err := pipeline.ConfigurePipeline(rr, expectedHash, p, pipelineResources, "test-promise", false, logger)
Describe("Resource Configure Pipeline", func() {
const expectedHash = "9bb58f26192e4ba00f01e2e7b136bbd8"
BeforeEach(func() {
pipelineResources = pipeline.NewPipelineArgs("test-promise", "test-promise-test-rr", "configure-step", "test-rr", "test-namespace")
job, err = pipeline.ConfigurePipeline(rr, expectedHash, p, pipelineResources, "test-promise", false, logger)
Expect(err).NotTo(HaveOccurred())

Expect(job.Labels).To(HaveKeyWithValue("kratix.io/hash", expectedHash))
labelsMatcher = MatchAllKeys(Keys{
"kratix.io/hash": Equal(expectedHash),
"kratix-workflow-action": Equal("configure"),
"kratix-workflow-pipeline-name": Equal("configure-step"),
"kratix.io/pipeline-name": Equal("configure-step"),
"kratix-workflow-type": Equal("resource"),
"kratix-workflow-kind": Equal("pipeline.platform.kratix.io"),
"kratix-workflow-promise-version": Equal("v1alpha1"),
"kratix.io/work-type": Equal("resource"),
"kratix.io/promise-name": Equal("test-promise"),
"kratix-promise-resource-request-id": Equal("test-promise-test-rr"),
"kratix.io/resource-name": Equal("test-rr"),
})
})

It("creates a job with the expected metadata", func() {
Expect(job.ObjectMeta).To(MatchFields(IgnoreExtras, Fields{
"Name": HavePrefix("kratix-test-promise-test-rr-configure-step-"),
"Namespace": Equal("test-namespace"),
"Labels": labelsMatcher,
}))
})

Context("when the pipeline name would exceed the 63 character limit", func() {
BeforeEach(func() {
promise_identifier := "long-long-long-long-promise"
pipelineResources = pipeline.NewPipelineArgs(promise_identifier, "long-long-long-long-promise-test-resource", "configure-step", "test-resource", "test-namespace")

job, err = pipeline.ConfigurePipeline(rr, expectedHash, p, pipelineResources, "test-promise", false, logger)

labelsMatcher = MatchAllKeys(Keys{
"kratix.io/hash": Equal(expectedHash),
"kratix-workflow-action": Equal("configure"),
"kratix-workflow-pipeline-name": Equal("configure-step"),
"kratix.io/pipeline-name": Equal("configure-step"),
"kratix-workflow-type": Equal("resource"),
"kratix-workflow-kind": Equal("pipeline.platform.kratix.io"),
"kratix-workflow-promise-version": Equal("v1alpha1"),
"kratix.io/work-type": Equal("resource"),
"kratix.io/promise-name": Equal(promise_identifier),
"kratix-promise-resource-request-id": Equal("long-long-long-long-promise-test-resource"),
"kratix.io/resource-name": Equal("test-resource"),
})
})

It("truncates the pipeline name to ensure it fits the 63 character limit", func() {
Expect(job.ObjectMeta.Name).To(HaveLen(62))
Expect(job.ObjectMeta).To(MatchFields(IgnoreExtras, Fields{
"Name": HavePrefix("kratix-long-long-long-long-promise-test-resource-configu-"),
"Namespace": Equal("test-namespace"),
"Labels": labelsMatcher,
}))
})
})
})

Expand Down
56 changes: 52 additions & 4 deletions lib/pipeline/delete_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ var _ = Describe("Delete Pipeline", func() {

Expect(job).To(MatchFields(IgnoreExtras, Fields{
"ObjectMeta": MatchFields(IgnoreExtras, Fields{
"Name": HavePrefix("delete-pipeline-custom-namespace-"),
"Name": HavePrefix("kratix-custom-namespace-promise-delete-"),
"Namespace": Equal("kratix-platform-system"),
"Labels": labelsMatcher,
}),
Expand Down Expand Up @@ -226,6 +226,29 @@ var _ = Describe("Delete Pipeline", func() {
}),
}))
})

Context("the pipeline name would exceed the 63 character limit", func() {
BeforeEach(func() {
promise := promiseFromFile(promisePath)
promise.SetName("long-long-long-long-long-long-promise")
unstructuredPromise, err := promise.ToUnstructured()
Expect(err).ToNot(HaveOccurred())

pipelines, err := promise.GeneratePipelines(logger)
Expect(err).ToNot(HaveOccurred())

pipelineResources = pipeline.NewDeletePromise(
unstructuredPromise,
pipelines.DeletePromise[0],
)
})

It("truncates the pipeline name to ensure it fits the 63 character limit", func() {
job = *pipelineResources[3].(*batchv1.Job)
Expect(job.ObjectMeta.Name).To(HaveLen(62))
Expect(job.ObjectMeta.Name).To(HavePrefix("kratix-long-long-long-long-long-long-promise-promise-del-"))
})
})
})
})

Expand All @@ -251,7 +274,7 @@ var _ = Describe("Delete Pipeline", func() {
pipelineResources = pipeline.NewDeleteResource(
resourceRequest,
pipelines.DeleteResource[0],
"example-custom-namespace",
"example-ns",
"custom-namespace",
"custom-namespaces",
)
Expand Down Expand Up @@ -325,7 +348,7 @@ var _ = Describe("Delete Pipeline", func() {
"kratix-workflow-type": Equal("resource"),
"kratix-workflow-action": Equal("delete"),
"kratix.io/promise-name": Equal("custom-namespace"),
"kratix-promise-resource-request-id": Equal("example-custom-namespace"),
"kratix-promise-resource-request-id": Equal("example-ns"),
"kratix-workflow-pipeline-name": Equal("instance-delete"),
"kratix.io/pipeline-name": Equal("instance-delete"),
"kratix.io/work-type": Equal("resource"),
Expand All @@ -334,7 +357,7 @@ var _ = Describe("Delete Pipeline", func() {

Expect(job).To(MatchFields(IgnoreExtras, Fields{
"ObjectMeta": MatchFields(IgnoreExtras, Fields{
"Name": HavePrefix("delete-pipeline-custom-namespace-"),
"Name": HavePrefix("kratix-custom-namespace-example-instance-delete-"),
"Namespace": Equal("default"),
"Labels": labelsMatcher,
}),
Expand Down Expand Up @@ -424,6 +447,31 @@ var _ = Describe("Delete Pipeline", func() {
}),
}))
})

Context("the pipeline name would exceed the 63 character limit", func() {
BeforeEach(func() {
promise := promiseFromFile(promisePath)
resourceRequest := resourceRequestFromFile(resourceRequestPath)
resourceRequest.SetName("long-long-request")

pipelines, err := promise.GeneratePipelines(logger)
Expect(err).ToNot(HaveOccurred())

pipelineResources = pipeline.NewDeleteResource(
resourceRequest,
pipelines.DeleteResource[0],
"long-long-request",
"long-long-promise",
"long-long-promises",
)
})

It("truncates the pipeline name to ensure it fits the 63 character limit", func() {
job = *pipelineResources[3].(*batchv1.Job)
Expect(job.ObjectMeta.Name).To(HaveLen(62))
Expect(job.ObjectMeta.Name).To(HavePrefix("kratix-long-long-promise-long-long-request-instance-dele-"))
})
})
})
})

Expand Down
18 changes: 10 additions & 8 deletions lib/pipeline/shared.go
Original file line number Diff line number Diff line change
@@ -1,17 +1,18 @@
package pipeline

import (
"fmt"
"os"
"strings"

"github.com/pkg/errors"
"github.com/syntasso/kratix/api/v1alpha1"
"github.com/syntasso/kratix/lib/resourceutil"
"gopkg.in/yaml.v2"
v1 "k8s.io/api/core/v1"
rbacv1 "k8s.io/api/rbac/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/util/uuid"
)

const (
Expand Down Expand Up @@ -213,12 +214,13 @@ func generateContainersAndVolumes(obj *unstructured.Unstructured, workflowType v
return containers, volumes
}

// TODO(breaking) change this to {promiseIdentifier}-{pipelineType}-pipeline-{short-uuid}
// for consistency with other resource names (e.g. service account)
func pipelineName(pipelineType, promiseIdentifier string) string {
return pipelineType + "-pipeline-" + promiseIdentifier + "-" + getShortUuid()
}
func pipelineName(promiseIdentifier, resourceIdentifier, objectName, pipelineName string) string {
var promiseResource = promiseIdentifier
if resourceIdentifier != "" {
promiseResource = fmt.Sprintf("%s-%s", promiseIdentifier, objectName)
}

pipelineIdentifier := fmt.Sprintf("kratix-%s-%s", promiseResource, pipelineName)

func getShortUuid() string {
return string(uuid.NewUUID()[0:5])
return resourceutil.GenerateObjectName(pipelineIdentifier)
}

0 comments on commit 097a721

Please sign in to comment.