Add Orchestrator ArgoCD and Tekton Plugin Integration#1375
Conversation
Reviewer's GuideThis pull request integrates GitOps for the Orchestrator by adding scripts to install ArgoCD and OpenShift Pipelines operators, provisioning ArgoCD AppProject and Tekton Tasks/Pipeline resources, and providing comprehensive documentation—while updating the orchestrator plugin to v1.6.0 and linking to the new GitOps guide. Sequence diagram for Orchestrator workflow deployment via Tekton Pipeline and ArgoCDsequenceDiagram
actor User
participant Tekton as Tekton Pipeline
participant Repo as Workflow Repo
participant GitOpsRepo as GitOps Repo
participant Quay as Quay.io
participant ArgoCD as ArgoCD AppProject
participant OpenShift as OpenShift Cluster
User->>Tekton: Trigger workflow-deployment pipeline
Tekton->>Repo: Clone workflow repository
Tekton->>Tekton: Flatten workflow, build manifests
Tekton->>Quay: Build & push workflow image
Tekton->>GitOpsRepo: Clone GitOps repo
Tekton->>GitOpsRepo: Update manifests, commit & push
Tekton->>ArgoCD: Push updated manifests
ArgoCD->>OpenShift: Deploy workflow resources
Entity relationship diagram for orchestrator-gitops namespace resourceserDiagram
NAMESPACE ||--o{ ARGOCD_PROJECT : contains
NAMESPACE ||--o{ TEKTON_TASK : contains
NAMESPACE ||--o{ TEKTON_PIPELINE : contains
NAMESPACE ||--o{ SECRET : contains
TEKTON_PIPELINE ||--o{ TEKTON_TASK : uses
TEKTON_PIPELINE ||--o{ SECRET : uses
ARGOCD_PROJECT ||--o{ APPLICATION : manages
SECRET {
string name
string type
}
TEKTON_TASK {
string name
string description
}
TEKTON_PIPELINE {
string name
string description
}
ARGOCD_PROJECT {
string name
string namespace
}
NAMESPACE {
string name
}
Class diagram for Tekton Pipeline and Tasks for Orchestrator GitOpsclassDiagram
class Pipeline {
+String name
+List<Task> tasks
+List<Workspace> workspaces
+List<Param> params
}
class Task {
+String name
+List<Step> steps
+List<Workspace> workspaces
+List<Param> params
}
class Step {
+String name
+String image
+String script
}
class Workspace {
+String name
+String description
}
class Param {
+String name
+String type
+String description
+String default
}
Pipeline "1" o-- "*" Task : uses
Task "1" o-- "*" Step : contains
Pipeline "1" o-- "*" Workspace : defines
Task "1" o-- "*" Workspace : defines
Pipeline "1" o-- "*" Param : defines
Task "1" o-- "*" Param : defines
class GitCliTask {
+git operations
+auth via SSH or basic-auth
+result: commit SHA
}
class FlattenerTask {
+flatten workflow structure
+param: workflowId
}
class BuildManifestsTask {
+generate manifests
+param: workflowId
}
class BuildGitOpsTask {
+update kustomize manifests
+param: workflowId, imageTag
}
Task <|-- GitCliTask
Task <|-- FlattenerTask
Task <|-- BuildManifestsTask
Task <|-- BuildGitOpsTask
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
f726f2e to
76e2840
Compare
76e2840 to
76e688e
Compare
There was a problem hiding this comment.
Hey @jenniferubah - I've reviewed your changes and found some issues that need to be addressed.
Blocking issues:
- Potential command injection risk with eval on user-supplied input. (link)
General comments:
- The delete operation currently removes the namespace first, causing subsequent resource deletions to fail—consider reversing the deletion order or explicitly handling namespace teardown last.
- You’ve hard-coded external URLs (Dockerfile, kn-workflow binary) and image versions in the script—parameterize or pin these values to ensure reproducible and upgrade-safe deployments.
- Embedding all Tekton YAML inside a single shell script makes maintenance harder—consider splitting them into separate template files or using a templating engine for clarity and easier updates.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The delete operation currently removes the namespace first, causing subsequent resource deletions to fail—consider reversing the deletion order or explicitly handling namespace teardown last.
- You’ve hard-coded external URLs (Dockerfile, kn-workflow binary) and image versions in the script—parameterize or pin these values to ensure reproducible and upgrade-safe deployments.
- Embedding all Tekton YAML inside a single shell script makes maintenance harder—consider splitting them into separate template files or using a templating engine for clarity and easier updates.
## Individual Comments
### Comment 1
<location> `config/profile/rhdh/plugin-infra/gitops/resources.sh:177` </location>
<code_context>
+ git config --global user.email "$(params.GIT_USER_EMAIL)"
+ git config --global user.name "$(params.GIT_USER_NAME)"
+
+ eval '$(params.GIT_SCRIPT)'
+
+ RESULT_SHA="$(git rev-parse HEAD | tr -d '\n')"
</code_context>
<issue_to_address>
Potential command injection risk with eval on user-supplied input.
Directly evaluating GIT_SCRIPT can execute arbitrary commands. Use safer alternatives or restrict input to mitigate this risk.
</issue_to_address>
### Comment 2
<location> `config/profile/rhdh/plugin-infra/gitops/resources.sh:160` </location>
<code_context>
+ set -x
+ fi
+
+ if [ "${WORKSPACE_BASIC_AUTH_DIRECTORY_BOUND}" = "true" ] ; then
+ cp "${WORKSPACE_BASIC_AUTH_DIRECTORY_PATH}/.git-credentials" "${PARAM_USER_HOME}/.git-credentials"
+ cp "${WORKSPACE_BASIC_AUTH_DIRECTORY_PATH}/.gitconfig" "${PARAM_USER_HOME}/.gitconfig"
+ chmod 400 "${PARAM_USER_HOME}/.git-credentials"
+ chmod 400 "${PARAM_USER_HOME}/.gitconfig"
+ fi
+
</code_context>
<issue_to_address>
Permissions set to 400 on .gitconfig may cause issues.
Using 400 may prevent git or other tools from modifying .gitconfig. Consider using 600, which is standard for config files.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
chmod 400 "${PARAM_USER_HOME}/.git-credentials"
chmod 400 "${PARAM_USER_HOME}/.gitconfig"
=======
chmod 400 "${PARAM_USER_HOME}/.git-credentials"
chmod 600 "${PARAM_USER_HOME}/.gitconfig"
>>>>>>> REPLACE
</suggested_fix>
### Comment 3
<location> `config/profile/rhdh/plugin-infra/gitops/resources.sh:167` </location>
<code_context>
+ chmod 400 "${PARAM_USER_HOME}/.gitconfig"
+ fi
+
+ if [ "${WORKSPACE_SSH_DIRECTORY_BOUND}" = "true" ] ; then
+ cp -R "${WORKSPACE_SSH_DIRECTORY_PATH}" "${PARAM_USER_HOME}"/.ssh
+ chmod 700 "${PARAM_USER_HOME}"/.ssh
+ chmod -R 400 "${PARAM_USER_HOME}"/.ssh/*
+ fi
+
</code_context>
<issue_to_address>
Recursive chmod 400 on all .ssh files may break SSH functionality.
Some .ssh files, like config or known_hosts, require less restrictive permissions. Set permissions based on file type to avoid SSH errors.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
chmod 700 "${PARAM_USER_HOME}"/.ssh
chmod -R 400 "${PARAM_USER_HOME}"/.ssh/*
fi
=======
chmod 700 "${PARAM_USER_HOME}"/.ssh
# Set private key permissions
find "${PARAM_USER_HOME}/.ssh" -type f \( -name "id_*" ! -name "*.pub" \) -exec chmod 400 {} \;
# Set public key permissions
find "${PARAM_USER_HOME}/.ssh" -type f -name "*.pub" -exec chmod 644 {} \;
# Set config and known_hosts permissions
[ -f "${PARAM_USER_HOME}/.ssh/config" ] && chmod 600 "${PARAM_USER_HOME}/.ssh/config"
[ -f "${PARAM_USER_HOME}/.ssh/known_hosts" ] && chmod 600 "${PARAM_USER_HOME}/.ssh/known_hosts"
fi
>>>>>>> REPLACE
</suggested_fix>
### Comment 4
<location> `config/profile/rhdh/plugin-infra/gitops/resources.sh:251` </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
+ ./kn-workflow gen-manifest --namespace ""
+---
+apiVersion: tekton.dev/v1
</code_context>
<issue_to_address>
No cleanup after installing packages with microdnf.
Run 'microdnf clean all' after installing packages to minimize image size and remove unnecessary cache files.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
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
./kn-workflow gen-manifest --namespace ""
=======
script: |
microdnf install -y tar gzip
microdnf clean all
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 ""
>>>>>>> REPLACE
</suggested_fix>
### Comment 5
<location> `config/profile/rhdh/plugin-infra/gitops/resources.sh:276` </location>
<code_context>
+ image: registry.access.redhat.com/ubi9-minimal
+ 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
</code_context>
<issue_to_address>
No check for existence of source or target directories before copy.
Add checks to ensure both source and target directories exist before running the cp command, or create the target directory if missing.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
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)
=======
script: |
SRC_DIR="$(workspaces.workflow-source.path)/flat/$(params.workflowId)/manifests"
DEST_DIR="kustomize/base"
if [ ! -d "$SRC_DIR" ]; then
echo "Source directory $SRC_DIR does not exist. Exiting."
exit 1
fi
if [ ! -d "$DEST_DIR" ]; then
echo "Target directory $DEST_DIR does not exist. Creating it."
mkdir -p "$DEST_DIR"
fi
cp "$SRC_DIR"/* "$DEST_DIR"
microdnf install -y findutils && microdnf clean all
cd kustomize
./updater.sh $(params.workflowId) $(params.imageTag)
>>>>>>> REPLACE
</suggested_fix>
### Comment 6
<location> `docs/orchestrator-gitops.md:168` </location>
<code_context>
+ssh-keygen -t rsa -b 4096 -f gitlab_ssh/id_rsa -N "" -C git@<GitLabHost> -q
+```
+
+- Add the SSH key to your GitLab account using the glab CLI or using the SSH keys setting:
+ You can find those setting in https://\<GitLabHost\>/-/user_settings/ssh_keys
+- Make sure to authenticate to your GitLab instance using your personal access token / Web authentication prior to using glab CLI.
+
</code_context>
<issue_to_address>
Typo: 'those setting' should be 'those settings'.
Change 'those setting' to 'those settings'.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
- Add the SSH key to your GitLab account using the glab CLI or using the SSH keys setting:
- You can find those setting in https://\<GitLabHost\>/-/user_settings/ssh_keys
=======
- Add the SSH key to your GitLab account using the glab CLI or using the SSH keys setting:
- You can find those settings in https://\<GitLabHost\>/-/user_settings/ssh_keys
>>>>>>> REPLACE
</suggested_fix>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey @jenniferubah - I've reviewed your changes and found some issues that need to be addressed.
Blocking issues:
- Potential command injection risk with eval on GIT_SCRIPT parameter. (link)
General comments:
- Externalize the embedded YAML for Tekton tasks and pipelines into separate manifest files instead of monolithic heredocs to improve readability and maintainability.
- Introduce configurable variables (namespace, image tags, CLI URLs) at the top of each script to avoid hardcoded values and simplify future upgrades.
- Consolidate the shared apply/delete logic in the Bash scripts by iterating over resource functions instead of duplicating the same calls in both branches.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Externalize the embedded YAML for Tekton tasks and pipelines into separate manifest files instead of monolithic heredocs to improve readability and maintainability.
- Introduce configurable variables (namespace, image tags, CLI URLs) at the top of each script to avoid hardcoded values and simplify future upgrades.
- Consolidate the shared apply/delete logic in the Bash scripts by iterating over resource functions instead of duplicating the same calls in both branches.
## Individual Comments
### Comment 1
<location> `config/profile/rhdh/plugin-infra/gitops/resources.sh:177` </location>
<code_context>
+ git config --global user.email "$(params.GIT_USER_EMAIL)"
+ git config --global user.name "$(params.GIT_USER_NAME)"
+
+ eval '$(params.GIT_SCRIPT)'
+
+ RESULT_SHA="$(git rev-parse HEAD | tr -d '\n')"
</code_context>
<issue_to_address>
Potential command injection risk with eval on GIT_SCRIPT parameter.
Consider replacing eval with a safer execution method or validating GIT_SCRIPT to prevent command injection from untrusted input.
</issue_to_address>
### Comment 2
<location> `config/profile/rhdh/plugin-infra/gitops/resources.sh:251` </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
+ ./kn-workflow gen-manifest --namespace ""
+---
+apiVersion: tekton.dev/v1
</code_context>
<issue_to_address>
No cleanup of installed packages in build-manifests task.
Installing tar and gzip without cleanup increases image size and attack surface. Please run 'microdnf clean all' after installation to minimize these risks.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
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
./kn-workflow gen-manifest --namespace ""
=======
script: |
microdnf install -y tar gzip
microdnf clean all
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 ""
>>>>>>> REPLACE
</suggested_fix>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
|
gazarenkov
left a comment
There was a problem hiding this comment.
Approved, take a look at a couple naming notes (not critical)
Co-authored-by: jenniferubah <jenniferubah@users.noreply.github.com>
Co-authored-by: jenniferubah <jenniferubah@users.noreply.github.com>
Co-authored-by: jenniferubah <jenniferubah@users.noreply.github.com>
|
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: gazarenkov The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Description
Adding script and documentation to setup ArgoCD and Tekton Pipeline for Orchestrator workflow.
Which issue(s) does this PR fix or relate to
PR acceptance criteria
How to test changes / Special notes to the reviewer
Summary by Sourcery
Add a GitOps infrastructure setup script to bootstrap ArgoCD and a Tekton pipeline for orchestrator workflows, supporting both apply and delete actions.
New Features:
Summary by Sourcery
Enable GitOps integration for the Orchestrator plugin by introducing scripts, Tekton resources, and documentation to automate ArgoCD and Tekton pipeline setup and manage workflow deployments.
New Features:
Enhancements:
Documentation: