Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure the configured TargetNamespace overrides the default namespace in the Dashboard's container args #2542

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Ensure the configured TargetNamespace overrides the default namespace in the Dashboard's container args #2542

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
97819ea
Refactor results details out of common transformer
aThorp96 Jan 27, 2025
e050a91
Apply TargetNamespace to Dashboard contaner args
aThorp96 Jan 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 12 additions & 13 deletions pkg/reconciler/common/transformers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,11 @@ const (
HubImagePrefix = "IMAGE_HUB_"
DashboardImagePrefix = "IMAGE_DASHBOARD_"

DefaultTargetNamespace = "tekton-pipelines"

ArgPrefix = "arg_"
ParamPrefix = "param_"

resultAPIDeployment = "tekton-results-api"
resultWatcherDeployment = "tekton-results-watcher"

runAsNonRootValue = true
allowPrivilegedEscalationValue = false
)
Expand Down Expand Up @@ -472,7 +471,7 @@ func injectNamespaceClusterRole(targetNamespace string) mf.Transformer {
if containsNamespaceResource && ok {
nm := []interface{}{}
for _, rn := range resourceNames.([]interface{}) {
if rn.(string) == "tekton-pipelines" {
if rn.(string) == DefaultTargetNamespace {
nm = append(nm, targetNamespace)
} else {
nm = append(nm, rn)
Expand All @@ -485,10 +484,10 @@ func injectNamespaceClusterRole(targetNamespace string) mf.Transformer {
}
}

// ReplaceNamespaceInDeploymentEnv replaces namespace in deployment's env var
func ReplaceNamespaceInDeploymentEnv(targetNamespace string) mf.Transformer {
// ReplaceNamespaceInDeploymentEnv replaces any instance of the default namespace string in the given deployments' env var
func ReplaceNamespaceInDeploymentEnv(deploymentNames []string, targetNamespace string) mf.Transformer {
return func(u *unstructured.Unstructured) error {
if u.GetKind() != "Deployment" || !(u.GetName() == resultAPIDeployment || u.GetName() == resultWatcherDeployment) {
if u.GetKind() != "Deployment" || !slices.Contains(deploymentNames, u.GetName()) {
return nil
}

Expand Down Expand Up @@ -516,16 +515,16 @@ func replaceNamespaceInDBAddress(envs []corev1.EnvVar, targetNamespace string) [

for i, e := range envs {
if slices.Contains(req, e.Name) {
envs[i].Value = strings.ReplaceAll(e.Value, "tekton-pipelines", targetNamespace)
envs[i].Value = strings.ReplaceAll(e.Value, DefaultTargetNamespace, targetNamespace)
}
}
return envs
}

// ReplaceNamespaceInDeploymentArgs replaces namespace in deployment's args
func ReplaceNamespaceInDeploymentArgs(targetNamespace string) mf.Transformer {
// ReplaceNamespaceInDeploymentArgs replaces any instance of the default namespace in the given deployments' args
func ReplaceNamespaceInDeploymentArgs(deploymentNames []string, targetNamespace string) mf.Transformer {
return func(u *unstructured.Unstructured) error {
if u.GetKind() != "Deployment" || u.GetName() != resultWatcherDeployment {
if u.GetKind() != "Deployment" || !slices.Contains(deploymentNames, u.GetName()) {
return nil
}

Expand All @@ -550,8 +549,8 @@ func ReplaceNamespaceInDeploymentArgs(targetNamespace string) mf.Transformer {

func replaceNamespaceInContainerArg(container *corev1.Container, targetNamespace string) {
for i, a := range container.Args {
if strings.Contains(a, "tekton-pipelines") {
container.Args[i] = strings.ReplaceAll(a, "tekton-pipelines", targetNamespace)
if strings.Contains(a, DefaultTargetNamespace) {
container.Args[i] = strings.ReplaceAll(a, DefaultTargetNamespace, targetNamespace)
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/reconciler/common/transformers_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -432,7 +432,7 @@ func TestReplaceNamespaceInDeploymentEnv(t *testing.T) {
manifest, err := mf.ManifestFrom(mf.Recursive(testData))
assertNoEror(t, err)

manifest, err = manifest.Transform(ReplaceNamespaceInDeploymentEnv("openshift-pipelines"))
manifest, err = manifest.Transform(ReplaceNamespaceInDeploymentEnv([]string{"tekton-results-watcher", "tekton-results-api"}, "openshift-pipelines"))
assertNoEror(t, err)

d := &appsv1.Deployment{}
Expand All @@ -456,7 +456,7 @@ func TestReplaceNamespaceInDeploymentArgs(t *testing.T) {
manifest, err := mf.ManifestFrom(mf.Recursive(testData))
assertNoEror(t, err)

manifest, err = manifest.Transform(ReplaceNamespaceInDeploymentArgs("openshift-pipelines"))
manifest, err = manifest.Transform(ReplaceNamespaceInDeploymentArgs([]string{"tekton-results-watcher"}, "openshift-pipelines"))
assertNoEror(t, err)

d := &appsv1.Deployment{}
Expand Down
6 changes: 3 additions & 3 deletions pkg/reconciler/kubernetes/tektondashboard/testdata/test-dashboard-transformer-updated.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -404,15 +404,15 @@ spec:
app.kubernetes.io/name: dashboard
app.kubernetes.io/part-of: tekton-dashboard
app.kubernetes.io/version: v0.48.0
operator.tekton.dev/deployment-spec-applied-hash: 0d7e625dccc2efab4493c409bdc71e88
operator.tekton.dev/deployment-spec-applied-hash: e521024acd6c6a280dc53fb4de0a6725
name: tekton-dashboard
spec:
containers:
- args:
- --port=9097
- --logout-url=
- --pipelines-namespace=tekton-pipelines
- --triggers-namespace=tekton-pipelines
- --pipelines-namespace=foo-ns
- --triggers-namespace=foo-ns
- --read-only=false
- --log-level=info
- --log-format=json
Expand Down
4 changes: 3 additions & 1 deletion pkg/reconciler/kubernetes/tektondashboard/transform.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ const (
func filterAndTransform(extension common.Extension) client.FilterAndTransform {
return func(ctx context.Context, manifest *mf.Manifest, comp v1alpha1.TektonComponent) (*mf.Manifest, error) {
dashboard := comp.(*v1alpha1.TektonDashboard)
targetNamespace := dashboard.Spec.GetTargetNamespace()

images := common.ToLowerCaseKeys(common.ImagesFromEnv(common.DashboardImagePrefix))

Expand All @@ -42,6 +43,7 @@ func filterAndTransform(extension common.Extension) client.FilterAndTransform {
common.AddConfiguration(dashboard.Spec.Config),
common.AddDeploymentRestrictedPSA(),
common.DeploymentImages(images),
common.ReplaceNamespaceInDeploymentArgs([]string{dashboardDeploymentName}, targetNamespace),
}
trns = append(trns, extra...)
if dashboard.Spec.ExternalLogs != "" {
Expand All @@ -54,7 +56,7 @@ func filterAndTransform(extension common.Extension) client.FilterAndTransform {

// additional options transformer
// always execute as last transformer, so that the values in options will be final update values on the manifests
if err := common.ExecuteAdditionalOptionsTransformer(ctx, manifest, dashboard.Spec.GetTargetNamespace(), dashboard.Spec.Dashboard.Options); err != nil {
if err := common.ExecuteAdditionalOptionsTransformer(ctx, manifest, targetNamespace, dashboard.Spec.Dashboard.Options); err != nil {
return &mf.Manifest{}, err
}

Expand Down
8 changes: 6 additions & 2 deletions pkg/reconciler/kubernetes/tektonresult/transform.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,13 @@ const (
loggingForwarderDelayDuration = "LOGGING_PLUGIN_FORWARDER_DELAY_DURATION"
logsAPIKey = "LOGS_API"
logsTypeKey = "LOGS_TYPE"

resultAPIDeployment = "tekton-results-api"
resultWatcherDeployment = "tekton-results-watcher"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if a new deployment is added to results component 🤔

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the previous implementation the new deployment name would need to have been added to pkg/reconciler/common/transformers.go's list of results deployments and to the check in ReplaceNamespaceInDeploymentEnv and ReplaceNamespaceInDeploymentArgs.

In this new implementation, the new deployment name would be added here and the the array resultDeploymentNames on line 64

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah make sense

)

var (
resultDeployementNames = []string{resultAPIDeployment, resultWatcherDeployment}
// allowed property secret keys
allowedPropertySecretKeys = []string{
"S3_BUCKET_NAME",
Expand All @@ -81,8 +85,8 @@ func (r *Reconciler) transform(ctx context.Context, manifest *mf.Manifest, comp
extra := []mf.Transformer{
common.InjectOperandNameLabelOverwriteExisting(v1alpha1.OperandTektoncdResults),
common.ApplyProxySettings,
common.ReplaceNamespaceInDeploymentArgs(targetNs),
common.ReplaceNamespaceInDeploymentEnv(targetNs),
common.ReplaceNamespaceInDeploymentArgs([]string{resultWatcherDeployment}, targetNs),
common.ReplaceNamespaceInDeploymentEnv(resultDeployementNames, targetNs),
updateApiConfig(instance.Spec),
enablePVCLogging(instance.Spec.ResultsAPIProperties),
updateEnvWithSecretName(instance.Spec.ResultsAPIProperties),
Expand Down