Skip to content

Commit

Permalink
Merge pull request #357 from rcerven/additionalParams
Browse files Browse the repository at this point in the history 
new option "additional-params" in build-pipeline-config configmap, which
  • Loading branch information
@rcerven
rcerven authored Oct 15, 2024
2 parents bb5d480 + b56441e commit 7049676
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 7049676

Please sign in to comment.