Orchestrator-software-templates chart: Post-RHDH install resources#199
Conversation
Reviewer's GuideThis PR adds a new Helm chart and accompanying setup script to automate post-RHDH installation steps, including secret generation, namespace labeling, and provisioning of Tekton tasks, pipelines and ArgoCD resources for orchestrator software templates. Sequence diagram for orchestrator-templates-setup.sh post-install automationsequenceDiagram
actor Admin
participant Script as orchestrator-templates-setup.sh
participant K8s as Kubernetes Cluster
participant RHDH as RHDH
participant ArgoCD
participant Tekton
participant GitHub
participant GitLab
Admin->>Script: Run setup script
Script->>K8s: Check prerequisites (oc)
Script->>K8s: Capture namespaces (RHDH, workflow, ArgoCD)
Script->>K8s: Generate secrets (backend, tokens)
Script->>K8s: Create or update ServiceAccounts
Script->>K8s: Create orchestrator-auth-secret
Script->>K8s: Label namespaces
Script->>K8s: Create ConfigMaps for RHDH
Script->>ArgoCD: Capture ArgoCD instance and credentials
Script->>Tekton: Prepare for pipeline execution
Script->>GitHub: Capture GitHub token/credentials
Script->>GitLab: Capture GitLab token/credentials
Script-->>Admin: Output setup completion
Class diagram for orchestrator-software-templates Helm chart resourcesclassDiagram
class OrchestratorSoftwareTemplatesChart {
+values.yaml
+templates/
+setup.sh
}
class TektonTask {
+git-cli
+flattener
+build-manifests
+build-gitops
}
class TektonPipeline {
+workflow-deployment
+params
+workspaces
+tasks
}
class ArgoCDProject {
+AppProject
+destinations
+sourceRepos
}
class ConfigMap {
+orchestrator-auth
+orchestrator-catalog
}
class Secret {
+orchestrator-auth-secret
+BACKEND_SECRET
+K8S_CLUSTER_TOKEN
+GITHUB_TOKEN
+GITLAB_TOKEN
+ARGOCD_PASSWORD
}
OrchestratorSoftwareTemplatesChart --> TektonTask
OrchestratorSoftwareTemplatesChart --> TektonPipeline
OrchestratorSoftwareTemplatesChart --> ArgoCDProject
OrchestratorSoftwareTemplatesChart --> ConfigMap
OrchestratorSoftwareTemplatesChart --> Secret
ConfigMap --> RHDH
Secret --> RHDH
TektonPipeline --> TektonTask
ArgoCDProject --> RHDH
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey @elai-shalev - I've reviewed your changes and found some issues that need to be addressed.
Blocking issues:
- Granting cluster-admin to a service account is overly permissive. (link)
General comments:
- The setup script repeats similar prompt-and-default logic for each variable—refactor that into a single helper function to reduce boilerplate and improve maintainability.
- The script grants cluster-admin to the ServiceAccount without scoping—consider limiting permissions to the minimum required or at least warning users about the elevated privileges.
- The Helm helpers for Tekton version and resource lookups contain dense inline logic—extract repeated semver and lookup checks into smaller, well-named template functions to improve readability.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The setup script repeats similar prompt-and-default logic for each variable—refactor that into a single helper function to reduce boilerplate and improve maintainability.
- The script grants cluster-admin to the ServiceAccount without scoping—consider limiting permissions to the minimum required or at least warning users about the elevated privileges.
- The Helm helpers for Tekton version and resource lookups contain dense inline logic—extract repeated semver and lookup checks into smaller, well-named template functions to improve readability.
## Individual Comments
### Comment 1
<location> `hack/orchestrator-templates-setup.sh:85` </location>
<code_context>
+ oc create sa "$sa_name" -n "$sa_namespace"
+ fi
+
+ oc adm policy add-cluster-role-to-user cluster-admin -z $sa_name -n $sa_namespace
+ echo "Added cluster-admin role to '$sa_name' in '$sa_namespace'."
+
</code_context>
<issue_to_address>
Granting cluster-admin to a service account is overly permissive.
Use a more restrictive role that grants only the permissions required for the orchestrator, rather than full cluster-admin access.
</issue_to_address>
### Comment 2
<location> `hack/orchestrator-templates-setup.sh:99` </location>
<code_context>
+ kubernetes.io/service-account.name: orchestrator
+EOF
+
+ K8S_CLUSTER_TOKEN=$(oc -n $sa_namespace get secret/${sa_name} -o jsonpath='{.data.token}' | base64 -d )
+}
+
</code_context>
<issue_to_address>
Token retrieval may not work reliably with Kubernetes 1.24+.
Kubernetes 1.24+ no longer auto-creates service account token secrets. You may need to create the token secret manually and ensure the token is ready before use. Please update the logic accordingly.
</issue_to_address>
### Comment 3
<location> `charts/orchestrator-software-templates/templates/tekton/tekton-tasks.yaml:107` </location>
<code_context>
+ value: $(params.VERBOSE)
+ - name: PARAM_USER_HOME
+ value: $(params.USER_HOME)
+ - name: WORKSPACE_OUTPUT_PATH
+ value: $(workspaces.output.path)
+ - name: WORKSPACE_SSH_DIRECTORY_BOUND
+ value: $(workspaces.ssh-directory.bound)
</code_context>
<issue_to_address>
WORKSPACE_OUTPUT_PATH is set but not used in the script.
WORKSPACE_OUTPUT_PATH appears unused and may be a copy-paste artifact. Consider removing it to avoid confusion.
</issue_to_address>
### Comment 4
<location> `charts/orchestrator-software-templates/templates/tekton/tekton-tasks.yaml:216` </location>
<code_context>
+ image: registry.access.redhat.com/ubi9-minimal
+ workingDir: $(workspaces.workflow-source.path)/flat/$(params.workflowId)
+ script: |
+ microdnf install -y tar gzip
+ KN_CLI_URL="https://developers.redhat.com/content-gateway/file/pub/cgw/serverless-logic/1.35.0/kn-workflow-linux-amd64.tar.gz"
+ curl -L "$KN_CLI_URL" | tar -xz --no-same-owner && chmod +x kn-workflow-linux-amd64 && mv kn-workflow-linux-amd64 kn-workflow
</code_context>
<issue_to_address>
Installing packages at runtime increases step duration and network dependency.
Using a custom image with 'tar' and 'gzip' pre-installed can reduce pipeline execution time and eliminate unnecessary network dependencies.
Suggested implementation:
```
image: your-org/ubi9-minimal-tar-gzip:latest
workingDir: $(workspaces.workflow-source.path)/flat/$(params.workflowId)
script: |
KN_CLI_URL="https://developers.redhat.com/content-gateway/file/pub/cgw/serverless-logic/1.35.0/kn-workflow-linux-amd64.tar.gz"
curl -L "$KN_CLI_URL" | tar -xz --no-same-owner && chmod +x kn-workflow-linux-amd64 && mv kn-workflow-linux-amd64 kn-workflow
./kn-workflow gen-manifest --namespace ""
```
- You must build and publish a custom image (e.g., `your-org/ubi9-minimal-tar-gzip:latest`) based on `ubi9-minimal` with `tar` and `gzip` pre-installed.
- Update the image reference to your actual image repository and tag.
</issue_to_address>
### Comment 5
<location> `charts/orchestrator-software-templates/templates/tekton/tekton-tasks.yaml:242` </location>
<code_context>
+ workingDir: $(workspaces.workflow-gitops.path)/workflow-gitops
+ script: |
+ cp $(workspaces.workflow-source.path)/flat/$(params.workflowId)/manifests/* kustomize/base
+ microdnf install -y findutils && microdnf clean all
+ cd kustomize
+ ./updater.sh $(params.workflowId) $(params.imageTag)
</code_context>
<issue_to_address>
Runtime installation of findutils could be avoided.
Consider switching to a base image with 'findutils' pre-installed to improve pipeline speed and reliability.
Suggested implementation:
```
image: registry.access.redhat.com/ubi9
```
```
cd kustomize
./updater.sh $(params.workflowId) $(params.imageTag)
```
</issue_to_address>
### Comment 6
<location> `charts/orchestrator-software-templates/templates/tekton/tekton-pipeline.yaml:170` </location>
<code_context>
+
+ cd workflow-gitops
+ git add .
+ git diff
+ # TODO: create PR
+ git commit -m "Deployment for workflow commit $WORKFLOW_COMMIT from $(params.gitUrl)"
+ # TODO: parametrize branch
</code_context>
<issue_to_address>
Pipeline pushes directly to main branch without PR or branch parameterization.
This approach could result in unreviewed changes reaching production. Please address the TODOs to require PRs and branch parameterization as soon as possible.
</issue_to_address>
### Comment 7
<location> `charts/orchestrator-software-templates/templates/tekton/tekton-pipeline.yaml:47` </location>
<code_context>
+ - name: ssh-directory
+ workspace: ssh-creds
+ params:
+ - name: GIT_USER_NAME
+ value: The Orchestrator Tekton Pipeline
+ - name: GIT_USER_EMAIL
+ value: rhdhorchestrator@redhat.com
</code_context>
<issue_to_address>
Hardcoded Git user/email may not be appropriate for all environments.
Consider making the Git user name and email configurable through pipeline parameters or environment variables.
</issue_to_address>
### Comment 8
<location> `charts/orchestrator-software-templates/templates/_helpers.tpl:118` </location>
<code_context>
+{{- define "get-argocd-namespace" -}}
+ {{- if .Values.argocd.enabled }}
+ {{- if (not (hasKey . "argoCDNamespace" ) ) -}}
+ #{{- $argoCDNamespace := include "get-namespace-with-label" (list .Values.argocd.namespace "rhdh.redhat.com/argocd-namespace") }}
+ #{{- $_ := set . "argoCDNamespace" $argoCDNamespace }}
+ {{- $_ := set . "argoCDNamespace" .Values.argocd.argocdNamespace }}
+ {{- end -}}
</code_context>
<issue_to_address>
Commented-out code for ArgoCD namespace selection may cause confusion.
Consider removing the commented-out namespace selection logic or add a note explaining its purpose to avoid confusion.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| oc create sa "$sa_name" -n "$sa_namespace" | ||
| fi | ||
|
|
||
| oc adm policy add-cluster-role-to-user cluster-admin -z $sa_name -n $sa_namespace |
There was a problem hiding this comment.
🚨 issue (security): Granting cluster-admin to a service account is overly permissive.
Use a more restrictive role that grants only the permissions required for the orchestrator, rather than full cluster-admin access.
| kubernetes.io/service-account.name: orchestrator | ||
| EOF | ||
|
|
||
| K8S_CLUSTER_TOKEN=$(oc -n $sa_namespace get secret/${sa_name} -o jsonpath='{.data.token}' | base64 -d ) |
There was a problem hiding this comment.
issue (bug_risk): Token retrieval may not work reliably with Kubernetes 1.24+.
Kubernetes 1.24+ no longer auto-creates service account token secrets. You may need to create the token secret manually and ensure the token is ready before use. Please update the logic accordingly.
| - name: WORKSPACE_OUTPUT_PATH | ||
| value: $(workspaces.output.path) |
There was a problem hiding this comment.
nitpick: WORKSPACE_OUTPUT_PATH is set but not used in the script.
WORKSPACE_OUTPUT_PATH appears unused and may be a copy-paste artifact. Consider removing it to avoid confusion.
| image: registry.access.redhat.com/ubi9-minimal | ||
| workingDir: $(workspaces.workflow-source.path)/flat/$(params.workflowId) | ||
| script: | | ||
| microdnf install -y tar gzip |
There was a problem hiding this comment.
suggestion (performance): Installing packages at runtime increases step duration and network dependency.
Using a custom image with 'tar' and 'gzip' pre-installed can reduce pipeline execution time and eliminate unnecessary network dependencies.
Suggested implementation:
image: your-org/ubi9-minimal-tar-gzip:latest
workingDir: $(workspaces.workflow-source.path)/flat/$(params.workflowId)
script: |
KN_CLI_URL="https://developers.redhat.com/content-gateway/file/pub/cgw/serverless-logic/1.35.0/kn-workflow-linux-amd64.tar.gz"
curl -L "$KN_CLI_URL" | tar -xz --no-same-owner && chmod +x kn-workflow-linux-amd64 && mv kn-workflow-linux-amd64 kn-workflow
./kn-workflow gen-manifest --namespace ""
- You must build and publish a custom image (e.g.,
your-org/ubi9-minimal-tar-gzip:latest) based onubi9-minimalwithtarandgzippre-installed. - Update the image reference to your actual image repository and tag.
| workingDir: $(workspaces.workflow-gitops.path)/workflow-gitops | ||
| script: | | ||
| cp $(workspaces.workflow-source.path)/flat/$(params.workflowId)/manifests/* kustomize/base | ||
| microdnf install -y findutils && microdnf clean all |
There was a problem hiding this comment.
suggestion (performance): Runtime installation of findutils could be avoided.
Consider switching to a base image with 'findutils' pre-installed to improve pipeline speed and reliability.
Suggested implementation:
image: registry.access.redhat.com/ubi9
cd kustomize
./updater.sh $(params.workflowId) $(params.imageTag)
| git diff | ||
| # TODO: create PR |
There was a problem hiding this comment.
issue (bug_risk): Pipeline pushes directly to main branch without PR or branch parameterization.
This approach could result in unreviewed changes reaching production. Please address the TODOs to require PRs and branch parameterization as soon as possible.
| - name: GIT_USER_NAME | ||
| value: The Orchestrator Tekton Pipeline |
There was a problem hiding this comment.
suggestion: Hardcoded Git user/email may not be appropriate for all environments.
Consider making the Git user name and email configurable through pipeline parameters or environment variables.
| #{{- $argoCDNamespace := include "get-namespace-with-label" (list .Values.argocd.namespace "rhdh.redhat.com/argocd-namespace") }} | ||
| #{{- $_ := set . "argoCDNamespace" $argoCDNamespace }} |
There was a problem hiding this comment.
nitpick: Commented-out code for ArgoCD namespace selection may cause confusion.
Consider removing the commented-out namespace selection logic or add a note explaining its purpose to avoid confusion.
There was a problem hiding this comment.
ShellCheck found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
|
|
|
924c5ee to
f49bbf3
Compare
|
|
12888fc to
dbf506a
Compare
|
|
a7b4f51 to
bba8d1e
Compare
|
|
b08b6db to
275c8ba
Compare
|
|
aa79930 to
7673e91
Compare
5003d07 to
06ec72e
Compare
|
b753b42
into
redhat-developer:main



This PR will introduce the new helm chart "Orchestrator-Software-templates" which will be responsible for adding the orchestrator software templates to the RHDH catalog of an existing rhdh deployment. This chart will also create authentication config maps, update dynamic plugins, and give instructions for configuring and running software templates on the catalog, using ArgoCD and Tekton for building and deploying the software projects that were created by the templates, and run the software project with Orchestrator plugin, via RHDH.
This proccess is a multistage proccess that includes many manual steps. This Helm Chart eases many of the steps, and refferes to the outsourced steps in the README and in the NOTES.txt.
This Chart is related to this Epic: https://issues.redhat.com/browse/RHIDP-7592
Checklist
Chart.yamlaccording to Semantic Versioning.values.yamland added to the corresponding README.md. The pre-commit utility can be used to generate the necessary content. Usepre-commit run -ato apply changes. The pre-commit Workflow will do this automatically for you if needed.pre-commithook.ct lintcommand.Summary by Sourcery
Introduce a new Helm chart to automate the integration of workflow software templates into an existing RHDH deployment and provide a setup script for post-install resource configuration.
New Features: