Skip to content

Commit

Permalink
chore: clarify varible naming and remove stale comments
Browse files Browse the repository at this point in the history
  • Loading branch information
SaphMB committed Jun 14, 2024
1 parent 2c80f5e commit 1e60178
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 14 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(promiseIdentifier, resourceRequestIdentifier, name, pName),
"delete-pipeline-name": pipelineName(promiseIdentifier, resourceRequestIdentifier, name, pName),
"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
4 changes: 2 additions & 2 deletions lib/pipeline/configure_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ var _ = Describe("Configure Pipeline", func() {
})
})

It("concatenates the pipeline name to ensure it fits the 63 character limit", func() {
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-"),
Expand Down Expand Up @@ -169,7 +169,7 @@ var _ = Describe("Configure Pipeline", func() {
})
})

It("concatenates the pipeline name to ensure it fits the 63 character limit", func() {
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-"),
Expand Down
4 changes: 2 additions & 2 deletions lib/pipeline/delete_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ var _ = Describe("Delete Pipeline", func() {
)
})

It("concatenates the pipeline name to ensure it fits the 63 character limit", func() {
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 Down Expand Up @@ -466,7 +466,7 @@ var _ = Describe("Delete Pipeline", func() {
)
})

It("concatenates the pipeline name to ensure it fits the 63 character limit", func() {
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
10 changes: 4 additions & 6 deletions lib/pipeline/shared.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,15 +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(promiseIdentifier, resourceIdentifier, name, pipelineName string) string {
var promise_resource = promiseIdentifier
func pipelineName(promiseIdentifier, resourceIdentifier, objectName, pipelineName string) string {
var promiseResource = promiseIdentifier
if resourceIdentifier != "" {
promise_resource = fmt.Sprintf("%s-%s", promiseIdentifier, name)
promiseResource = fmt.Sprintf("%s-%s", promiseIdentifier, objectName)
}

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

return resourceutil.GenerateObjectName(pipelineIdentifier)
}

0 comments on commit 1e60178

Please sign in to comment.