Skip to content

feat(HUM-2068): push-rpms integration test update#2238

Merged
swickersh merged 1 commit into
konflux-ci:developmentfrom
scoheb:HUM-2068-push-rpms-integration-test
May 29, 2026
Merged

feat(HUM-2068): push-rpms integration test update#2238
swickersh merged 1 commit into
konflux-ci:developmentfrom
scoheb:HUM-2068-push-rpms-integration-test

Conversation

@scoheb

@scoheb scoheb commented May 20, 2026

Copy link
Copy Markdown
Member

Summary

  • Update RPA with signOptions config for RPM signing (keytab, pipeline image, SA, artifact storage, key alias, unsigned/signed domains)
  • Add multi-component test support with wait_for_releases using internal helper
  • Update vault secrets for new Pulp bot and packages URL
  • Enhance create-pulp-resources script with signed repo support
  • Use unsigned domain for artifact verification

Test plan

  • Integration test: cd integration-tests && ./run-test.sh push-rpms-to-pulp -i

Jira

https://redhat.atlassian.net/browse/HUM-2068

Part of HUM-959 - PR 6 of 6

🤖 Generated with Claude Code

@qodo-code-review

Copy link
Copy Markdown
Contributor

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

Comment thread integration-tests/push-rpms-to-pulp/resources/managed/rpa.yaml
Comment thread integration-tests/push-rpms-to-pulp/resources/managed/rpa.yaml
Comment thread integration-tests/push-rpms-to-pulp/test.sh Outdated
Comment thread integration-tests/push-rpms-to-pulp/test.sh
@scoheb scoheb force-pushed the HUM-2068-push-rpms-integration-test branch 2 times, most recently from c326a82 to c9cec58 Compare May 25, 2026 13:38
@scoheb

scoheb commented May 25, 2026

Copy link
Copy Markdown
Member Author

The push-rpms-to-pulp-e2e-test failure is expected — this PR updates the integration test to reference the RPM signing workflow, but the required task/pipeline changes are in PRs #2233#2237 which haven't been merged to development yet. This PR should be merged last in the sequence, after which the e2e test will pass.

@scoheb scoheb force-pushed the HUM-2068-push-rpms-integration-test branch from e32198d to 535ae40 Compare May 25, 2026 16:29
swickersh
swickersh previously approved these changes May 26, 2026

@swickersh swickersh 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
#2237 needs approved/merged first.

@scoheb scoheb force-pushed the HUM-2068-push-rpms-integration-test branch from 535ae40 to 31f789e Compare May 26, 2026 14:53
@scoheb scoheb requested a review from theflockers May 26, 2026 14:55
@swickersh

Copy link
Copy Markdown
Contributor

this will need prod/needs-approval label to progress through review process once @theflockers is good with the change.

@scoheb scoheb force-pushed the HUM-2068-push-rpms-integration-test branch from 31f789e to a928eb8 Compare May 26, 2026 18:23
@scoheb scoheb requested a review from swickersh May 26, 2026 18:28
swickersh
swickersh previously approved these changes May 26, 2026

@swickersh swickersh 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.

looks like just a vault update since last approval.
lgtm

@scoheb scoheb force-pushed the HUM-2068-push-rpms-integration-test branch 2 times, most recently from 6d08b12 to cfde158 Compare May 29, 2026 16:02
@qodo-code-review

qodo-code-review Bot commented May 29, 2026

Copy link
Copy Markdown
Contributor

PR Reviewer Guide 🔍

(Review updated until commit ba4b0ee)

Warning

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

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

HUM-2068 - Partially compliant

Compliant requirements:

  • Update push-rpms-to-pulp integration test to support/validate the RPM signing workflow.
  • Ensure integration test configuration supports new signing-related inputs (e.g., sign options / domains) as needed for the workflow.

Non-compliant requirements:

Requires further human verification:

  • Validate the updated integration test via an integration run (./run-test.sh push-rpms-to-pulp -i).

HUM-959 - Partially compliant

Compliant requirements:

Non-compliant requirements:

Requires further human verification:

  • Contribute the rpm-signing branch changes back to release-service-catalog development branch.
  • Ensure the set of related PRs/subtasks are integrated/compatible as part of the overall effort.
⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 PR contains tests
🔒 Security concerns

Supply chain risk:
signPipelineImage is set to quay.io/konflux-ci/signing:latest, which is mutable and could pull unexpected content over time. Prefer a pinned digest/tag to reduce the risk of unintended execution changes.

⚡ Recommended focus areas for review

Unused var

release_names is assigned but the script iterates over RELEASE_NAMES. This may be a leftover from a refactor and can cause confusion about which variable is authoritative for multi-release behavior.

    release_names="${release_name}"

    "${SUITE_DIR}/../scripts/wait-for-release.sh"
}
YQ selection

The new yq expressions select the first matching src artifact via [0]. If no src artifact exists, this can yield null/error behavior depending on yq version; consider guarding with a length check and/or using -r to ensure consistent empty-string output.

sbom_url=$(yq '[.spec.content.artifacts[] | select(.architecture == "src")] | .[0].sbom // ""' \
  "${advisory_yaml_dir}/advisory.yaml")

if [ -n "${sbom_url}" ]; then
  echo "Found SBOM URL: ${sbom_url}"

  # Get Pulp credentials from the secret
  # Disable tracing to prevent credential exposure
  { set +x; } 2>/dev/null
  pulp_secret_name="pulp-credentials-${component_name}"
  cli_toml=$(kubectl get secret "${pulp_secret_name}" -n "${managed_namespace}" \
    -o jsonpath='{.data.cli\.toml}' 2>/dev/null | base64 -d || echo "")

  if [ -n "${cli_toml}" ]; then
    pulp_username=$(echo "${cli_toml}" | grep username | cut -d'"' -f2)
    pulp_password=$(echo "${cli_toml}" | grep password | cut -d'"' -f2)

    # Try to download the SBOM
    sbom_file=$(mktemp)
    http_code=$(curl -L -s -w "%{http_code}" -u "${pulp_username}:${pulp_password}" \
      -o "${sbom_file}" "${sbom_url}" 2>/dev/null || echo "000")

    if [ "${http_code}" = "200" ]; then
      echo "✅️ SBOM downloaded successfully (HTTP ${http_code})"
      # Validate it's valid JSON
      if jq -e . "${sbom_file}" >/dev/null 2>&1; then
        echo "✅️ SBOM is valid JSON"
      else
        echo "🔴 SBOM is not valid JSON"
        failures=$((failures+1))
      fi
    else
      echo "🔴 Failed to download SBOM (HTTP ${http_code})"
      failures=$((failures+1))
    fi
    rm -f "${sbom_file}"
  else
    echo "Warning: Could not retrieve Pulp credentials, skipping SBOM download verification"
  fi
else
  echo "🔴 sbom field not found in source RPM artifact entry"
  failures=$((failures+1))
fi

# Verify attestation field is present for source RPM artifacts and is downloadable
echo "Checking attestation for source RPM artifacts..."
attestation_url=$(yq '[.spec.content.artifacts[] | select(.architecture == "src")] | .[0].attestation // ""' \
  "${advisory_yaml_dir}/advisory.yaml")
Pinned image

signPipelineImage uses a :latest tag, which can make the integration test non-reproducible and flaky if the image changes. Prefer pinning to a digest or a specific version tag.

signPipelineImage: "quay.io/konflux-ci/signing:latest"
signPipelineServiceAccount: "signing-pipeline-sa"
# Skips the signing pipeline's re-upload of signed RPMs to Pulp;

@qodo-code-review

Copy link
Copy Markdown
Contributor

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 integration-tests/push-rpms-to-pulp/resources/managed/rpa.yaml
theflockers
theflockers previously approved these changes May 29, 2026

@theflockers theflockers 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.

my comments were clarified. So LGTM.

@scoheb scoheb dismissed stale reviews from theflockers and swickersh via 13d09a3 May 29, 2026 16:40
@scoheb scoheb force-pushed the HUM-2068-push-rpms-integration-test branch from cfde158 to 13d09a3 Compare May 29, 2026 16:40
@qodo-code-review

Copy link
Copy Markdown
Contributor

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 integration-tests/push-rpms-to-pulp/test.sh Outdated
Comment thread integration-tests/push-rpms-to-pulp/test.sh
@scoheb scoheb requested review from swickersh and theflockers May 29, 2026 16:54
@scoheb scoheb force-pushed the HUM-2068-push-rpms-integration-test branch from 13d09a3 to 5a3793b Compare May 29, 2026 18:59
@qodo-code-review

Copy link
Copy Markdown
Contributor

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 integration-tests/push-rpms-to-pulp/test.sh Outdated
Comment thread integration-tests/push-rpms-to-pulp/resources/managed/rpa.yaml
Comment thread integration-tests/push-rpms-to-pulp/resources/managed/rpa.yaml
swickersh
swickersh previously approved these changes May 29, 2026
@johnbieren

Copy link
Copy Markdown
Collaborator

This is not a user facing change, so no prod-approval label needed. I removed it. Scott H, you can resolve the qodo comments yourself after either addressing them or deciding they aren't necessary. Scott W, you can merge this once it has passing e2e

@scoheb scoheb force-pushed the HUM-2068-push-rpms-integration-test branch from 5a3793b to 879a42f Compare May 29, 2026 19:56
Update RPA with signOptions config for RPM signing (keytab, pipeline
image, SA, artifact storage, key alias, unsigned/signed domains).
Add multi-component test support with wait_for_releases using
internal helper. Update vault secrets for new Pulp bot and packages
URL. Enhance create-pulp-resources script with signed repo support.
Use unsigned domain for artifact verification.

- Add comments clarifying RPA sign options
- Use yq array indexing instead of piping to head -n1 to prevent
  SIGPIPE (exit 141) when multiple source RPM artifacts are returned

Signed-off-by: Scott Hebert <shebert@redhat.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@scoheb scoheb force-pushed the HUM-2068-push-rpms-integration-test branch from 879a42f to ba4b0ee Compare May 29, 2026 19:56
@qodo-code-review

Copy link
Copy Markdown
Contributor

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 integration-tests/push-rpms-to-pulp/test.sh
Comment thread integration-tests/push-rpms-to-pulp/test.sh
Comment thread integration-tests/push-rpms-to-pulp/resources/managed/rpa.yaml
@qodo-code-review

Copy link
Copy Markdown
Contributor

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 integration-tests/push-rpms-to-pulp/test.sh
Comment thread integration-tests/push-rpms-to-pulp/test.sh
Comment thread integration-tests/push-rpms-to-pulp/resources/managed/rpa.yaml
@swickersh

Copy link
Copy Markdown
Contributor

This is not a user facing change, so no prod-approval label needed. I removed it. Scott H, you can resolve the qodo comments yourself after either addressing them or deciding they aren't necessary. Scott W, you can merge this once it has passing e2e

Ack, thanks. Sorry forgot we don't need the label on these.

@swickersh swickersh added this pull request to the merge queue May 29, 2026
Merged via the queue into konflux-ci:development with commit 043dd9a May 29, 2026
31 checks passed
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.

4 participants