Skip to content

Commit

Permalink
new option "additional-params" in build-pipeline-config configmap, which
Browse files Browse the repository at this point in the history 
is list with params to be included with default values (from pipeline
spec) to the created pipeline runs

STONEBLD-2789

Signed-off-by: Robert Cerven <[email protected]>
  • Loading branch information
@rcerven
rcerven committed Oct 14, 2024
1 parent bb5d480 commit b56441e
Show file tree
Hide file tree
Showing 5 changed files with 82 additions and 55 deletions.
2 changes: 1 addition & 1 deletion controllers/component_build_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ func (r *ComponentBuildReconciler) Reconcile(ctx context.Context, req ctrl.Reque
return ctrl.Result{}, nil
}

_, err = r.GetBuildPipelineFromComponentAnnotation(ctx, &component)
_, _, err = r.GetBuildPipelineFromComponentAnnotation(ctx, &component)
if err != nil {
buildStatus := readBuildStatus(&component)
// when reading pipeline annotation fails, we should end reconcile, unless transient error
Expand Down
82 changes: 53 additions & 29 deletions controllers/component_build_controller_pipeline.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,9 @@ import (
)

type BuildPipeline struct {
Name string `json:"name,omitempty"`
Bundle string `json:"bundle,omitempty"`
Name string `json:"name,omitempty"`
Bundle string `json:"bundle,omitempty"`
AdditionalParams []string `json:"additional-params,omitempty"`
}

type pipelineConfig struct {
Expand Down Expand Up @@ -89,10 +90,11 @@ func (r *ComponentBuildReconciler) generatePaCPipelineRunConfigs(ctx context.Con
var pipelineName string
var pipelineBundle string
var pipelineRef *tektonapi.PipelineRef
var additionalParams []string
var err error

// no need to check error because it would fail already in Reconcile
pipelineRef, _ = r.GetBuildPipelineFromComponentAnnotation(ctx, component)
pipelineRef, additionalParams, _ = r.GetBuildPipelineFromComponentAnnotation(ctx, component)
pipelineName, pipelineBundle, err = getPipelineNameAndBundle(pipelineRef)
if err != nil {
return nil, nil, err
Expand All @@ -108,7 +110,7 @@ func (r *ComponentBuildReconciler) generatePaCPipelineRunConfigs(ctx context.Con
return nil, nil, err
}

pipelineRunOnPush, err := generatePaCPipelineRunForComponent(component, pipelineSpec, pacTargetBranch, gitClient, false)
pipelineRunOnPush, err := generatePaCPipelineRunForComponent(component, pipelineSpec, additionalParams, pacTargetBranch, gitClient, false)
if err != nil {
return nil, nil, err
}
Expand All @@ -117,7 +119,7 @@ func (r *ComponentBuildReconciler) generatePaCPipelineRunConfigs(ctx context.Con
return nil, nil, err
}

pipelineRunOnPR, err := generatePaCPipelineRunForComponent(component, pipelineSpec, pacTargetBranch, gitClient, true)
pipelineRunOnPR, err := generatePaCPipelineRunForComponent(component, pipelineSpec, additionalParams, pacTargetBranch, gitClient, true)
if err != nil {
return nil, nil, err
}
Expand Down Expand Up @@ -166,47 +168,49 @@ func retrievePipelineSpec(ctx context.Context, bundleUri, pipelineName string) (
}

// GetBuildPipelineFromComponentAnnotation parses pipeline annotation on component and returns build pipeline
func (r *ComponentBuildReconciler) GetBuildPipelineFromComponentAnnotation(ctx context.Context, component *appstudiov1alpha1.Component) (*tektonapi.PipelineRef, error) {
func (r *ComponentBuildReconciler) GetBuildPipelineFromComponentAnnotation(ctx context.Context, component *appstudiov1alpha1.Component) (*tektonapi.PipelineRef, []string, error) {
buildPipeline, err := readBuildPipelineAnnotation(component)
if err != nil {
return nil, err
return nil, nil, err
}
if buildPipeline == nil {
err := fmt.Errorf("missing or empty pipeline annotation: %s, will add default one to the component", component.Annotations[defaultBuildPipelineAnnotation])
return nil, boerrors.NewBuildOpError(boerrors.EMissingPipelineAnnotation, err)
return nil, nil, boerrors.NewBuildOpError(boerrors.EMissingPipelineAnnotation, err)
}
if buildPipeline.Bundle == "" || buildPipeline.Name == "" {
err = fmt.Errorf("missing name or bundle in pipeline annotation: name=%s bundle=%s", buildPipeline.Name, buildPipeline.Bundle)
return nil, boerrors.NewBuildOpError(boerrors.EWrongPipelineAnnotation, err)
return nil, nil, boerrors.NewBuildOpError(boerrors.EWrongPipelineAnnotation, err)
}
finalBundle := buildPipeline.Bundle
additionalParams := []string{}

if buildPipeline.Bundle == "latest" {
pipelinesConfigMap := &corev1.ConfigMap{}
if err := r.Client.Get(ctx, types.NamespacedName{Name: buildPipelineConfigMapResourceName, Namespace: BuildServiceNamespaceName}, pipelinesConfigMap); err != nil {
if errors.IsNotFound(err) {
return nil, boerrors.NewBuildOpError(boerrors.EBuildPipelineConfigNotDefined, err)
}
return nil, err
pipelinesConfigMap := &corev1.ConfigMap{}
if err := r.Client.Get(ctx, types.NamespacedName{Name: buildPipelineConfigMapResourceName, Namespace: BuildServiceNamespaceName}, pipelinesConfigMap); err != nil {
if errors.IsNotFound(err) {
return nil, nil, boerrors.NewBuildOpError(boerrors.EBuildPipelineConfigNotDefined, err)
}
return nil, nil, err
}

buildPipelineData := &pipelineConfig{}
if err := yaml.Unmarshal([]byte(pipelinesConfigMap.Data[buildPipelineConfigName]), buildPipelineData); err != nil {
return nil, boerrors.NewBuildOpError(boerrors.EBuildPipelineConfigNotValid, err)
}
buildPipelineData := &pipelineConfig{}
if err := yaml.Unmarshal([]byte(pipelinesConfigMap.Data[buildPipelineConfigName]), buildPipelineData); err != nil {
return nil, nil, boerrors.NewBuildOpError(boerrors.EBuildPipelineConfigNotValid, err)
}

for _, pipeline := range buildPipelineData.Pipelines {
if pipeline.Name == buildPipeline.Name {
for _, pipeline := range buildPipelineData.Pipelines {
if pipeline.Name == buildPipeline.Name {
if buildPipeline.Bundle == "latest" {
finalBundle = pipeline.Bundle
break
}
additionalParams = pipeline.AdditionalParams
break
}
}

// requested pipeline was not found in configMap
if finalBundle == "latest" {
err = fmt.Errorf("invalid pipeline name in pipeline annotation: name=%s", buildPipeline.Name)
return nil, boerrors.NewBuildOpError(boerrors.EBuildPipelineInvalid, err)
}
// requested pipeline was not found in configMap
if finalBundle == "latest" {
err = fmt.Errorf("invalid pipeline name in pipeline annotation: name=%s", buildPipeline.Name)
return nil, nil, boerrors.NewBuildOpError(boerrors.EBuildPipelineInvalid, err)
}

pipelineRef := &tektonapi.PipelineRef{
Expand All @@ -219,7 +223,7 @@ func (r *ComponentBuildReconciler) GetBuildPipelineFromComponentAnnotation(ctx c
},
},
}
return pipelineRef, nil
return pipelineRef, additionalParams, nil
}

func readBuildPipelineAnnotation(component *appstudiov1alpha1.Component) (*BuildPipeline, error) {
Expand Down Expand Up @@ -276,6 +280,7 @@ func (r *ComponentBuildReconciler) SetDefaultBuildPipelineComponentAnnotation(ct
func generatePaCPipelineRunForComponent(
component *appstudiov1alpha1.Component,
pipelineSpec *tektonapi.PipelineSpec,
additionalParams []string,
pacTargetBranch string,
gitClient gp.GitProviderClient,
onPull bool) (*tektonapi.PipelineRun, error) {
Expand Down Expand Up @@ -328,6 +333,25 @@ func generatePaCPipelineRunForComponent(
params = append(params, tektonapi.Param{Name: "image-expires-after", Value: tektonapi.ParamValue{Type: "string", StringVal: prImageExpiration}})
}

for _, additionalParam := range additionalParams {
for _, pipelineParam := range pipelineSpec.Params {
if additionalParam == pipelineParam.Name {
if pipelineParam.Type == "string" {
params = append(params, tektonapi.Param{Name: additionalParam, Value: tektonapi.ParamValue{Type: "string", StringVal: pipelineParam.Default.StringVal}})
break
}
if pipelineParam.Type == "array" {
params = append(params, tektonapi.Param{Name: additionalParam, Value: tektonapi.ParamValue{Type: "array", ArrayVal: pipelineParam.Default.ArrayVal}})
break
}
if pipelineParam.Type == "object" {
params = append(params, tektonapi.Param{Name: additionalParam, Value: tektonapi.ParamValue{Type: "object", ObjectVal: pipelineParam.Default.ObjectVal}})
break
}
}
}
}

if component.Spec.Source.GitSource.DockerfileURL != "" {
params = append(params, tektonapi.Param{Name: "dockerfile", Value: tektonapi.ParamValue{Type: "string", StringVal: component.Spec.Source.GitSource.DockerfileURL}})
} else {
Expand Down
12 changes: 4 additions & 8 deletions controllers/component_build_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ var _ = Describe("Component initial build controller", func() {
createNamespace(pipelinesAsCodeNamespace)
createRoute(pacRouteKey, "pac-host")
createNamespace(BuildServiceNamespaceName)
createDefaultBuildPipelineConfigMap(defaultPipelineConfigMapKey)
pacSecretData := map[string]string{
"github-application-id": "12345",
"github-private-key": githubAppPrivateKey,
Expand Down Expand Up @@ -253,7 +254,6 @@ var _ = Describe("Component initial build controller", func() {
return nil
}

createDefaultBuildPipelineConfigMap(defaultPipelineConfigMapKey)
annotations := map[string]string{}
createCustomComponentWithBuildRequest(componentConfig{
componentKey: resourcePacPrepKey,
Expand All @@ -276,7 +276,6 @@ var _ = Describe("Component initial build controller", func() {
}, timeout, interval).Should(BeTrue())

expectPacBuildStatus(resourcePacPrepKey, "enabled", 0, "", mergeUrl)
deleteBuildPipelineConfigMap(defaultPipelineConfigMapKey)
})

It("should successfully submit PR with PaC definitions using GitHub application, using 'latest' bundle", func() {
Expand Down Expand Up @@ -305,7 +304,6 @@ var _ = Describe("Component initial build controller", func() {
return nil
}

createDefaultBuildPipelineConfigMap(defaultPipelineConfigMapKey)
annotationValue := fmt.Sprintf("{\"name\":\"%s\",\"bundle\":\"%s\"}", defaultPipelineName, "latest")
annotations := map[string]string{defaultBuildPipelineAnnotation: annotationValue}
createCustomComponentWithBuildRequest(componentConfig{
Expand All @@ -329,7 +327,6 @@ var _ = Describe("Component initial build controller", func() {
}, timeout, interval).Should(BeTrue())

expectPacBuildStatus(resourcePacPrepKey, "enabled", 0, "", mergeUrl)
deleteBuildPipelineConfigMap(defaultPipelineConfigMapKey)
})

It("should fail to submit PR if build pipeline annotation isn't valid json", func() {
Expand All @@ -349,7 +346,6 @@ var _ = Describe("Component initial build controller", func() {
})

It("should fail to submit PR if build pipeline annotation has non existing pipeline", func() {
createDefaultBuildPipelineConfigMap(defaultPipelineConfigMapKey)
annotationValue := fmt.Sprintf("{\"name\":\"%s\",\"bundle\":\"%s\"}", "wrong-pipeline", "latest")
annotations := map[string]string{defaultBuildPipelineAnnotation: annotationValue}
createCustomComponentWithBuildRequest(componentConfig{
Expand All @@ -364,11 +360,9 @@ var _ = Describe("Component initial build controller", func() {
Expect(buildStatus).ToNot(BeNil())
errorMessage := fmt.Sprintf("%d: %s", expectError.GetErrorId(), expectError.ShortError())
Expect(buildStatus.Message).To(ContainSubstring(errorMessage))
deleteBuildPipelineConfigMap(defaultPipelineConfigMapKey)
})

It("should fail to submit PR if build pipeline annotation is missing bundle and name", func() {
createDefaultBuildPipelineConfigMap(defaultPipelineConfigMapKey)
annotationValue := "{\"some\":\"wrong\"}"
annotations := map[string]string{defaultBuildPipelineAnnotation: annotationValue}
createCustomComponentWithBuildRequest(componentConfig{
Expand All @@ -383,7 +377,6 @@ var _ = Describe("Component initial build controller", func() {
Expect(buildStatus).ToNot(BeNil())
errorMessage := fmt.Sprintf("%d: %s", expectError.GetErrorId(), expectError.ShortError())
Expect(buildStatus.Message).To(ContainSubstring(errorMessage))
deleteBuildPipelineConfigMap(defaultPipelineConfigMapKey)
})

It("should fail to submit PR if GitHub application is not installed into git repository", func() {
Expand Down Expand Up @@ -724,6 +717,7 @@ var _ = Describe("Component initial build controller", func() {
createNamespace(pipelinesAsCodeNamespace)
createRoute(pacRouteKey, "pac-host")
createNamespace(BuildServiceNamespaceName)
createDefaultBuildPipelineConfigMap(defaultPipelineConfigMapKey)

ResetTestGitProviderClient()
})
Expand Down Expand Up @@ -1203,6 +1197,7 @@ var _ = Describe("Component initial build controller", func() {
createNamespace(pipelinesAsCodeNamespace)
createRoute(pacRouteKey, "pac-host")
createNamespace(BuildServiceNamespaceName)
createDefaultBuildPipelineConfigMap(defaultPipelineConfigMapKey)
ResetTestGitProviderClient()

pacSecretData := map[string]string{
Expand Down Expand Up @@ -1302,6 +1297,7 @@ var _ = Describe("Component initial build controller", func() {
createNamespace(pipelinesAsCodeNamespace)
createRoute(pacRouteKey, "pac-host")
createNamespace(BuildServiceNamespaceName)
createDefaultBuildPipelineConfigMap(defaultPipelineConfigMapKey)
pacSecretData := map[string]string{
"github-application-id": "12345",
"github-private-key": githubAppPrivateKey,
Expand Down
26 changes: 23 additions & 3 deletions controllers/component_build_controller_unit_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,8 @@ func TestGeneratePaCPipelineRunForComponent(t *testing.T) {
},
Status: appstudiov1alpha1.ComponentStatus{},
}
param1_value := "param1_value"
param2_value := []string{"param2_value1", "param2_value2"}
pipelineSpec := &tektonapi.PipelineSpec{
Workspaces: []tektonapi.PipelineWorkspaceDeclaration{
{
Expand All @@ -224,11 +226,15 @@ func TestGeneratePaCPipelineRunForComponent(t *testing.T) {
Name: "workspace",
},
},
Params: tektonapi.ParamSpecs{
{Name: "add-param1", Type: "string", Default: &tektonapi.ParamValue{Type: "string", StringVal: param1_value}},
{Name: "add-param2", Type: "array", Default: &tektonapi.ParamValue{Type: "array", ArrayVal: param2_value}},
},
}
branchName := "custom-branch"
ResetTestGitProviderClient()

pipelineRun, err := generatePaCPipelineRunForComponent(component, pipelineSpec, branchName, testGitProviderClient, true)
pipelineRun, err := generatePaCPipelineRunForComponent(component, pipelineSpec, []string{"add-param1", "add-param2", "non-existing"}, branchName, testGitProviderClient, true)
if err != nil {
t.Error("generatePaCPipelineRunForComponent(): Failed to genertate pipeline run")
}
Expand Down Expand Up @@ -270,7 +276,7 @@ func TestGeneratePaCPipelineRunForComponent(t *testing.T) {
t.Errorf("generatePaCPipelineRunForComponent(): wrong build.appstudio.redhat.com/pull_request_number annotation value")
}

if len(pipelineRun.Spec.Params) != 6 {
if len(pipelineRun.Spec.Params) != 8 {
t.Error("generatePaCPipelineRunForComponent(): wrong number of pipeline params")
}
for _, param := range pipelineRun.Spec.Params {
Expand Down Expand Up @@ -299,6 +305,20 @@ func TestGeneratePaCPipelineRunForComponent(t *testing.T) {
if param.Value.StringVal != "base_context" {
t.Errorf("generatePaCPipelineRunForComponent(): wrong pipeline parameter %s value", param.Name)
}
case "add-param1":
if param.Value.StringVal != param1_value {
t.Errorf("generatePaCPipelineRunForComponent(): wrong pipeline parameter %s value", param.Name)
}
case "add-param2":
if len(param.Value.ArrayVal) != len(param2_value) {
t.Errorf("generatePaCPipelineRunForComponent(): wrong pipeline parameter %s value", param.Name)
}
for idx := range param2_value {
if param2_value[idx] != param.Value.ArrayVal[idx] {
t.Errorf("generatePaCPipelineRunForComponent(): wrong pipeline parameter %s value", param.Name)

}
}
default:
t.Errorf("generatePaCPipelineRunForComponent(): unexpected pipeline parameter %v", param)
}
Expand All @@ -319,7 +339,7 @@ func TestGeneratePaCPipelineRunForComponent(t *testing.T) {
}

func TestGeneratePaCPipelineRunForComponent_ShouldStopIfTargetBranchIsNotSet(t *testing.T) {
_, err := generatePaCPipelineRunForComponent(nil, nil, "", nil, true)
_, err := generatePaCPipelineRunForComponent(nil, nil, nil, "", nil, true)
if err == nil {
t.Errorf("generatePaCPipelineRunForComponent(): expected error")
}
Expand Down
15 changes: 1 addition & 14 deletions controllers/suite_util_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -564,7 +564,7 @@ func createBuildPipelineConfigMap(configMapKey types.NamespacedName, pipelineBun
configMapData := map[string]string{}
buildPipelineData := pipelineConfig{
DefaultPipelineName: pipelineName,
Pipelines: []BuildPipeline{{Name: pipelineName, Bundle: pipelineBundle}},
Pipelines: []BuildPipeline{{Name: pipelineName, Bundle: pipelineBundle, AdditionalParams: []string{"additional_param1"}}},
}
yamlData, _ := yaml.Marshal(&buildPipelineData)
configMapData[buildPipelineConfigName] = string(yamlData)
Expand All @@ -579,19 +579,6 @@ func createBuildPipelineConfigMap(configMapKey types.NamespacedName, pipelineBun
}
}

func deleteBuildPipelineConfigMap(configMapKey types.NamespacedName) {
buildPipelineConfigMap := corev1.ConfigMap{}
if err := k8sClient.Get(ctx, configMapKey, &buildPipelineConfigMap); err != nil {
if k8sErrors.IsNotFound(err) {
return
}
Fail(err.Error())
}
if err := k8sClient.Delete(ctx, &buildPipelineConfigMap); err != nil && !k8sErrors.IsNotFound(err) {
Fail(err.Error())
}
}

func waitServiceAccount(serviceAccountKey types.NamespacedName) corev1.ServiceAccount {
serviceAccount := corev1.ServiceAccount{}
Eventually(func() bool {
Expand Down

0 comments on commit b56441e

Please sign in to comment.