Skip to content

refactor(RELEASE-1985): convert create-advisory internal task to python#2244

Merged
johnbieren merged 1 commit into
konflux-ci:developmentfrom
johnbieren:release1985
Jun 15, 2026
Merged

refactor(RELEASE-1985): convert create-advisory internal task to python#2244
johnbieren merged 1 commit into
konflux-ci:developmentfrom
johnbieren:release1985

Conversation

@johnbieren

Copy link
Copy Markdown
Collaborator

This commit replaces the inline bash script for the create-advisory internal task with a standalone python script contained in the utils image. The tekton unit tests are updated accordingly (mocks converted to a way that works with the python script and test scenarios removed that are already covered via pytest in the utils repo).

Describe your changes

Relevant Jira

Checklist before requesting a review

  • I have marked as draft or added do not merge label if there's a dependency PR
    • If you want reviews on your draft PR, you can add reviewers or add the release-service-maintainers handle if you are unsure who to tag
  • My commit message includes Signed-off-by: My name <email>
  • I read CONTRIBUTING.MD and commit formatting
  • I have run the README.md generator script in .github/scripts/readme_generator.sh and verified the results using .github/scripts/check_readme.sh
  • If an AI agent was used, I marked that via a commit footer like Assisted-By: Cursor

@johnbieren

Copy link
Copy Markdown
Collaborator Author

/retest

@johnbieren

Copy link
Copy Markdown
Collaborator Author

/retest

@johnbieren

Copy link
Copy Markdown
Collaborator Author

/retest

@johnbieren

Copy link
Copy Markdown
Collaborator Author

/retest

@johnbieren

Copy link
Copy Markdown
Collaborator Author

/retest

@johnbieren

Copy link
Copy Markdown
Collaborator Author

/retest

2 similar comments
@johnbieren

Copy link
Copy Markdown
Collaborator Author

/retest

@johnbieren

Copy link
Copy Markdown
Collaborator Author

/retest

@johnbieren

Copy link
Copy Markdown
Collaborator Author

/retest

@johnbieren

Copy link
Copy Markdown
Collaborator Author

/retest

@johnbieren johnbieren force-pushed the release1985 branch 6 times, most recently from 077d321 to 5578df0 Compare May 29, 2026 17:59
@johnbieren

Copy link
Copy Markdown
Collaborator Author

Depends on konflux-ci/release-service-utils#778

@johnbieren johnbieren force-pushed the release1985 branch 2 times, most recently from 0bfa911 to 05ffafe Compare May 29, 2026 18:50
@johnbieren johnbieren force-pushed the release1985 branch 2 times, most recently from fb67b6d to 9b228ab Compare June 3, 2026 13:29
@johnbieren johnbieren marked this pull request as ready for review June 3, 2026 13:29
@johnbieren

Copy link
Copy Markdown
Collaborator Author

/retest

@johnbieren johnbieren force-pushed the release1985 branch 2 times, most recently from 845d7a8 to 69388e4 Compare June 10, 2026 11:57
@johnbieren

Copy link
Copy Markdown
Collaborator Author

/retest

@johnbieren

Copy link
Copy Markdown
Collaborator Author

/retest

This commit replaces the inline bash script for the create-advisory
internal task with a standalone python script contained in the utils
image. The tekton unit tests are updated accordingly (mocks converted to
a way that works with the python script and test scenarios removed that
are already covered via pytest in the utils repo).

Assisted-By: Cursor

Signed-off-by: Johnny Bieren <jbieren@redhat.com>
@johnbieren johnbieren marked this pull request as ready for review June 10, 2026 12:18
@qodo-app-for-konflux-ci

Copy link
Copy Markdown

PR Reviewer Guide 🔍

Warning

/review is deprecated. Use /agentic_review instead (removal date not yet scheduled).

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Interface Drift

The task now passes params/results via multiple PARAM_* and RESULT_* environment variables and switches to a Python entrypoint. Please validate that the Python script’s expected environment variable names match these exactly (including casing) and that all previously produced Tekton results are still written for downstream consumers.

- name: create-advisory
  image: quay.io/konflux-ci/release-service-utils@sha256:71a99d12d920fcc157e08e5dc9894fbc4bce42328e9c2f58dc53411278858d91
  computeResources:
    limits:
      memory: 512Mi
    requests:
      memory: 512Mi
      cpu: '1'  # 1 is the max allowed by at least the staging cluster
  volumeMounts:
    - name: advisory-secret
      mountPath: /mnt/advisory_secret
    - name: errata-secret
      mountPath: /mnt/errata_secret
  env:
    - name: ADVISORY_JSON
      value: $(params.advisory_json)
    - name: PARAM_COMPONENT_GROUP
      value: $(params.componentGroup)
    - name: PARAM_ORIGIN
      value: $(params.origin)
    - name: PARAM_CONFIG_MAP_NAME
      value: $(params.config_map_name)
    - name: PARAM_CONTENT_TYPE
      value: $(params.contentType)
    - name: PARAM_INTERNAL_REQUEST_PIPELINE_RUN_NAME
      value: $(params.internalRequestPipelineRunName)
    - name: PARAM_TASK_RUN_NAME
      value: $(context.taskRun.name)
    - name: RESULT_RESULT
      value: $(results.result.path)
    - name: RESULT_ADVISORY_URL
      value: $(results.advisory_url.path)
    - name: RESULT_ADVISORY_INTERNAL_URL
      value: $(results.advisory_internal_url.path)
    - name: RESULT_INTERNAL_REQUEST_PIPELINE_RUN_NAME
      value: $(results.internalRequestPipelineRunName.path)
    - name: RESULT_INTERNAL_REQUEST_TASK_RUN_NAME
      value: $(results.internalRequestTaskRunName.path)
  command: ["/home/scripts/python/tasks/internal/create_advisory.py"]
Test Robustness

The pre-apply hook creates cluster-scoped test fixtures (RBAC, ConfigMap, Secrets) and writes a temp file under /tmp. Consider making the script more robust by ensuring cleanup (e.g., trap-based teardown) and using stricter bash settings so partial failures don’t leave state behind that could affect subsequent test runs.

# Add RBAC so that the SA executing the tests can retrieve CRs
kubectl apply -f .github/resources/crd_rbac.yaml

cat > "/tmp/cm.json" << EOF
{
    "apiVersion": "v1",
    "data": {
        "PYXIS_URL": "https://pyxis.stage.engineering.redhat.com",
        "SIG_KEY_NAMES": "redhate2etesting redhate2etesting2",
        "PYXIS_SSL_CERT_FILE_NAME": "hacbs-signing-pipeline.pem",
        "PYXIS_SSL_CERT_SECRET_NAME": "hacbs-signing-pipeline-certs",
        "PYXIS_SSL_KEY_FILE_NAME": "hacbs-signing-pipeline.key",
        "UMB_CLIENT_NAME": "hacbs-signing-pipeline-nonprod",
        "UMB_LISTEN_TOPIC": "VirtualTopic.eng.robosignatory.hacbs.sign",
        "UMB_PUBLISH_TOPIC": "VirtualTopic.eng.hacbs-signing-pipeline.hacbs.sign",
        "UMB_URL": "umb.stage.api.redhat.com",
        "UMB_SSL_CERT_FILE_NAME": "hacbs-signing-pipeline.pem",
        "UMB_SSL_CERT_SECRET_NAME": "hacbs-signing-pipeline-certs",
        "UMB_SSL_KEY_FILE_NAME": "hacbs-signing-pipeline.key",
        "SIGNER_TYPE": "batch"
    },
    "kind": "ConfigMap",
    "metadata": {
        "name": "create-advisory-test-cm"
    }
}
EOF
kubectl delete -f /tmp/cm.json --ignore-not-found
kubectl create -f /tmp/cm.json

kubectl delete secret create-advisory-secret --ignore-not-found
kubectl create secret generic create-advisory-secret \
    --from-literal=git_author_email=tester@tester \
    --from-literal=git_author_name=tester \
    --from-literal=gitlab_access_token=abc \
    --from-literal=gitlab_host=myurl \
    --from-literal=git_repo=https://gitlab.com/org/repo.git

kubectl delete secret create-advisory-errata-secret --ignore-not-found
kubectl create secret generic create-advisory-errata-secret \
    --from-literal=errata_api=https://errata/api/v1 \
    --from-literal=name=errata-tester \
    --from-literal=base64_keytab=Zm9vCg==
📚 Focus areas based on broader codebase context

Task Semantics

This task previously documented (and relied on) the invariant that it "will always exit 0 even if something fails" so that task results are still set and propagated. With the refactor to a Python entrypoint, ensure the invoked script preserves that behavior (always writes all expected results and exits 0 even on errors), otherwise downstream internal-request handling may break. (Ref 1, Ref 4)

- name: create-advisory
  image: quay.io/konflux-ci/release-service-utils@sha256:71a99d12d920fcc157e08e5dc9894fbc4bce42328e9c2f58dc53411278858d91
  computeResources:
    limits:
      memory: 512Mi
    requests:
      memory: 512Mi
      cpu: '1'  # 1 is the max allowed by at least the staging cluster
  volumeMounts:
    - name: advisory-secret
      mountPath: /mnt/advisory_secret
    - name: errata-secret
      mountPath: /mnt/errata_secret
  env:
    - name: ADVISORY_JSON
      value: $(params.advisory_json)
    - name: PARAM_COMPONENT_GROUP
      value: $(params.componentGroup)
    - name: PARAM_ORIGIN
      value: $(params.origin)
    - name: PARAM_CONFIG_MAP_NAME
      value: $(params.config_map_name)
    - name: PARAM_CONTENT_TYPE
      value: $(params.contentType)
    - name: PARAM_INTERNAL_REQUEST_PIPELINE_RUN_NAME
      value: $(params.internalRequestPipelineRunName)
    - name: PARAM_TASK_RUN_NAME
      value: $(context.taskRun.name)
    - name: RESULT_RESULT
      value: $(results.result.path)
    - name: RESULT_ADVISORY_URL
      value: $(results.advisory_url.path)
    - name: RESULT_ADVISORY_INTERNAL_URL
      value: $(results.advisory_internal_url.path)
    - name: RESULT_INTERNAL_REQUEST_PIPELINE_RUN_NAME
      value: $(results.internalRequestPipelineRunName.path)
    - name: RESULT_INTERNAL_REQUEST_TASK_RUN_NAME
      value: $(results.internalRequestTaskRunName.path)
  command: ["/home/scripts/python/tasks/internal/create_advisory.py"]

Reference reasoning: The existing internal task definitions explicitly call out "always exit 0 even if something fails" as a key contract because task results must be set regardless of failures. The PR changes the implementation from an inline bash script (which trapped EXIT/ERR and forced exit 0 while writing results) to a Python command, so the new entrypoint must intentionally re-implement the same contract.

📄 References
  1. konflux-ci/release-service-catalog/tasks/internal/create-advisory-task/create-advisory-task.yaml [1-15]
  2. konflux-ci/release-service-catalog/tasks/internal/create-advisory-task/tests/test-create-advisory.yaml [1-13]
  3. konflux-ci/release-service-catalog/pipelines/internal/create-advisory/create-advisory.yaml [47-74]
  4. konflux-ci/release-service-catalog/tasks/internal/create-advisory-oci-artifact-task/create-advisory-oci-artifact-task.yaml [1-15]
  5. konflux-ci/release-service-catalog/pipelines/internal/create-advisory-oci-artifact/create-advisory-oci-artifact.yaml [23-38]
  6. konflux-ci/release-service-catalog/pipelines/managed/rh-rpm-advisories/rh-rpm-advisories.yaml [251-267]
  7. konflux-ci/release-service-catalog/tasks/managed/create-advisory/create-advisory.yaml [1-14]
  8. konflux-ci/release-service-catalog/pipelines/managed/calunga-push-to-pulp/calunga-push-to-pulp.yaml [346-355]

@qodo-app-for-konflux-ci

Copy link
Copy Markdown

PR Code Suggestions ✨

Warning

/improve is deprecated. Use /agentic_review instead (removal date not yet scheduled).

Inline suggestions were posted as code suggestions.

Comment thread tasks/internal/create-advisory-task/create-advisory-task.yaml
Comment thread tasks/internal/create-advisory-task/create-advisory-task.yaml
Comment thread .github/scripts/mock_http_json.py
@johnbieren

Copy link
Copy Markdown
Collaborator Author

@FilipNikolovski @seanconroy2021 this one is finally ready, PTAL

@FilipNikolovski FilipNikolovski left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, have one question - noticed you're only using env vars for the params instead of command line args for the script, is there a specific reason for that or its just a preference?

@johnbieren

Copy link
Copy Markdown
Collaborator Author

LGTM, have one question - noticed you're only using env vars for the params instead of command line args for the script, is there a specific reason for that or its just a preference?

Not a specific reason - that is just how cursor did it the first time and I stuck with it

@FilipNikolovski

Copy link
Copy Markdown
Contributor

LGTM, have one question - noticed you're only using env vars for the params instead of command line args for the script, is there a specific reason for that or its just a preference?

Not a specific reason - that is just how cursor did it the first time and I stuck with it

okay thanks!

@seanconroy2021 seanconroy2021 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

@ach912 ach912 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

lgtm

@ach912

ach912 commented Jun 15, 2026

Copy link
Copy Markdown

Production Approval Record

Field Value
Action APPROVED
Reviewer @ach912
Timestamp 2026-06-15T09:39:52.166Z

Approved

@johnbieren johnbieren added this pull request to the merge queue Jun 15, 2026
Merged via the queue into konflux-ci:development with commit b3fe7ae Jun 15, 2026
33 checks passed
@johnbieren johnbieren deleted the release1985 branch June 15, 2026 12:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants