diff --git a/lib/pipeline/args.go b/lib/pipeline/args.go index 206e662a..c2e5c83b 100644 --- a/lib/pipeline/args.go +++ b/lib/pipeline/args.go @@ -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, @@ -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{ diff --git a/lib/pipeline/configure_test.go b/lib/pipeline/configure_test.go index 1fc7d28d..c709f1f9 100644 --- a/lib/pipeline/configure_test.go +++ b/lib/pipeline/configure_test.go @@ -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" @@ -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() { @@ -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, + })) + }) }) }) diff --git a/lib/pipeline/delete_test.go b/lib/pipeline/delete_test.go index 8c4929dc..0062b2f5 100644 --- a/lib/pipeline/delete_test.go +++ b/lib/pipeline/delete_test.go @@ -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, }), @@ -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-")) + }) + }) }) }) @@ -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", ) @@ -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"), @@ -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, }), @@ -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-")) + }) + }) }) }) diff --git a/lib/pipeline/shared.go b/lib/pipeline/shared.go index 5bd3b2c0..2ed5cf3b 100644 --- a/lib/pipeline/shared.go +++ b/lib/pipeline/shared.go @@ -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 ( @@ -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) }