Skip to content

Adding publish-rpm-repository action/workflow - #16

Open
alexlovelltroy wants to merge 1 commit into
mainfrom
feature/publish-rpm-repository
Open

Adding publish-rpm-repository action/workflow#16
alexlovelltroy wants to merge 1 commit into
mainfrom
feature/publish-rpm-repository

Conversation

@alexlovelltroy

Copy link
Copy Markdown
Member

Description

This pull request introduces a new reusable GitHub Action and workflow for securely publishing RPM repositories to S3, including robust documentation and CI enhancements. The main focus is on enabling safe, verifiable, and automated publication of signed RPM packages, with strong key management and metadata integrity guarantees. The changes add both the action implementation and comprehensive usage documentation, as well as integration into the CI pipeline for testing.

New RPM Repository Publication Action and Workflow

Major new features and documentation:

1. New reusable workflow and action for RPM repository publication

  • Adds .github/workflows/publish-rpm-repository.yml and actions/publish-rpm-repository/action.yml to aggregate signed RPMs, generate and sign repository metadata, and publish to S3 with optional CloudFront invalidation. This includes concurrency controls, robust key handling, and a safe publication order to ensure metadata integrity. [1] [2]

2. Comprehensive documentation for the new publication process

  • Documents the action and workflow in README.md and a dedicated actions/publish-rpm-repository/README.md, covering publication model, security, inputs/outputs, and client trust configuration. Usage examples and security best practices are included. [1] [2] [3] [4]

3. CI/CD and linting enhancements

  • Updates .github/workflows/ci.yml to add a test-rpm-repository-publisher job that exercises the new publisher inside a container, and adds a new .github/actionlint.yaml to suppress false positives for workflow-specific properties. [1] [2]

4. Security and compliance

  • All new files include SPDX copyright and license headers, ensuring compliance and traceability. [1] [2] [3] [4]

5. README enhancements and cross-linking

  • Updates action and workflow listings in README.md for discoverability, and adds cross-references between documentation files for easier navigation. [1] [2] [3]

These changes collectively provide a robust, auditable, and easy-to-use solution for publishing signed RPM repositories in a secure and automated fashion, with clear guidance for both direct and workflow-based usage.

Checklist

  • My code follows the style guidelines of this project
  • I have added/updated comments where needed
  • I have added tests that prove my fix is effective or my feature works
  • I have run make test (or equivalent) locally and all tests pass
  • I have updated the relevant documentation (CLI examples, man pages, README, other docs, etc.)
  • DCO Sign-off: All commits are signed off (git commit -s) with my real name and email
  • REUSE Compliance:
    • Each new/modified source file has SPDX copyright and license headers
    • Any non-commentable files include a <filename>.license sidecar
    • All referenced licenses are present in the LICENSES/ directory

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update
  • Dependency update

For more info, see Contributing Guidelines.

Signed-off-by: Alex Lovell-Troy <alovelltroy@lanl.gov>

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

Overall this seems fine to me. Were you able to test it against one of our existing repos (example: I used metadata-service when testing the gpg signing workflows)?

Aside from that, I only came across some nitpicks and out-of-scope ideas for future edits. All optional.

Comment thread README.md
Comment on lines +17 to +25
- `actions/publish-rpm-repository`: Generates durable-key-signed repository metadata and safely publishes RPM repositories to S3
- `.github/workflows/go-build-release.yml`: Reusable workflow for GoReleaser builds
- `.github/workflows/docker-build-release.yml`: Reusable workflow for multi-arch container image builds
- `.github/workflows/build-publish-container-goreleaser.yml`: Builds and publishes a container image via GoReleaser
- `.github/workflows/build-rpm-quadlet.yml`: Builds a caller repo's podman quadlet RPM
- `.github/workflows/gpg-sign-artifacts.yml`: Signs unsigned RPM artifacts with a per-run ephemeral key
- `.github/workflows/validate-rpm-quadlet.yml`: Validates a signed quadlet RPM's installed file list
- `.github/workflows/release-signed-artifacts.yml`: Publishes a GitHub Release with signed RPMs and public keys
- `.github/workflows/publish-rpm-repository.yml`: Publishes aggregated RPMs through S3 and CloudFront using GitHub OIDC

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.

nitpick: it's a bit jarring for both the workflow and the action to have the same name.

Comment thread README.md
Comment on lines +229 to +237
repository-path: stable/el9/x86_64
s3-bucket: ${{ vars.RPM_REPOSITORY_BUCKET }}
aws-region: us-east-1
aws-role-arn: ${{ vars.RPM_REPOSITORY_PUBLISHER_ROLE_ARN }}
aws-account-id: ${{ vars.AWS_ACCOUNT_ID }}
cloudfront-distribution-id: ${{ vars.RPM_REPOSITORY_DISTRIBUTION_ID }}
public-base-url: https://rpm.openchami.org
package-master-fingerprint: ${{ vars.MASTER_FPR }}
metadata-signing-key-fingerprint: ${{ vars.RPM_REPOSITORY_SIGNING_KEY_FINGERPRINT }}

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.

Reminder for @seantronsen

We might want to create a github issue that focuses on standardizing varnames + usage across the recently created workflows + actions. This is out of scope for this PR, but if it's to happen it should occur before these are used in the many downstream repos under the org.

Comment on lines +102 to +105
steps:
- name: Checkout publisher action at the reusable workflow commit
uses: actions/checkout@9f698171ed81b15d1823a05fc7211befd50c8ae0 # v6.0.3
with:

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.

Is there a reason we are using the full ref associated with the tag and not just the tag itself? This repo doesn't appear to have a standard (some tags, some full refs).

Out of scope, but should we standardize later on? If so, I'll create a tracking issue.

Comment on lines +10 to +12
artifact-name-signed-rpms:
description: 'Name of the signed RPM artifact to publish'
type: string

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.

Grammar mismatch. Var name is plural, doc comment is singular.

Comment on lines +15 to +19
artifact-name-package-signing-keys:
description: 'Name of the certified ephemeral package public-key artifact'
type: string
default: 'public-keys'
required: false

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.

Ibid, see above grammar 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.

I do like this being an external script. We might want to refactor the other actions at some point to follow this design. Out of scope for now.

Comment on lines +19 to +34
rpm_path=${INPUT_RPM_PATH:?rpm-path is required}
repository_path=${INPUT_REPOSITORY_PATH:?repository-path is required}
s3_bucket=${INPUT_S3_BUCKET:-}
distribution_id=${INPUT_CLOUDFRONT_DISTRIBUTION_ID:-}
public_base_url=${INPUT_PUBLIC_BASE_URL:-}
package_keys_path=${INPUT_PACKAGE_SIGNING_KEYS_PATH:?package-signing-keys-path is required}
package_master_public_key=${INPUT_PACKAGE_MASTER_PUBLIC_KEY_ASC:?package master public key is required}
package_master_fingerprint=${INPUT_PACKAGE_MASTER_FINGERPRINT:?package master fingerprint is required}
authorized_package_repo_fingerprints=${INPUT_AUTHORIZED_PACKAGE_REPO_FINGERPRINTS:?authorized package repo fingerprints are required}
package_key_bundle_filename=${INPUT_PACKAGE_KEY_BUNDLE_FILENAME:-RPM-GPG-KEY-OpenCHAMI-Packages}
signing_key_b64=${INPUT_METADATA_SIGNING_KEY_ARMORED_B64:?metadata signing key is required}
signing_fingerprint=${INPUT_METADATA_SIGNING_KEY_FINGERPRINT:?metadata signing fingerprint is required}
signing_passphrase=${INPUT_METADATA_SIGNING_KEY_PASSPHRASE:-}
key_filename=${INPUT_REPOSITORY_KEY_FILENAME:-RPM-GPG-KEY-OpenCHAMI}
require_signed_rpms=$(normalize_bool "${INPUT_REQUIRE_SIGNED_RPMS:-true}")
dry_run=$(normalize_bool "${INPUT_DRY_RUN:-false}")

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.

nitpick on code style: I'll defer to your judgement on this, but to me this part seems unnecessarily verbose since the script already requires the env vars utilized here. if the goal is to simply shorten the var names for future usage, we could just shorten the env vars and reference them directly?

As for checking + normalizing, we could use a loop func for this. Pasting an example below from one of my external projects:

function check-variable-assignments {

    if [ "${#}" -eq 0 ]; then
        echo "error: check-variable-assignments received zero arguments" >&2
        return 1
    fi

    local return_status=0
    while [ "${#}" -ne 0 ]; do
        local name="${1}"
        if [[ -v "${name}" ]]; then
            if [[ -n "${!name}" ]]; then
                echo "${name}=${!name}" >&2
            else
                echo "${name}: set but empty" >&2
                return_status=1
            fi
        else
            echo "${name}: required, but unset" >&2
            return_status=1
        fi
        shift
    done

    return "${return_status}"
}


# usage

check-variable-assignments \
    CERTIFICATE_AUTHORITY_URL \
    CERTIFICATE_AUTHORITY_PROVISIONER \
    FILEPATH_CERTIFICATE_AUTHORITY_CERT \
    FILEPATH_CERTIFICATE_AUTHORITY_PASSWD \
    SVC_DOMAIN \
    FILEPATH_CERT \
    FILEPATH_KEY \
    FILEPATH_COMBO \
    FILEPATH_COMBO_PFX \
    EXPECTED_UID \
    EXPECTED_GID \
    SVC_KEY_TYPE \
    SVC_KEY_CURVE || exit

Note

This is a little bit longer, but I'd argue:

  1. it's more readable + maintainable in the long run
  2. easy to reuse
  3. it checks all var settings at the same time without erroring out until the end, meaning at runtime you'll discover all issues / typos simultaneously instead of one per execution.

Comment on lines +87 to +93
if [[ "$dry_run" == 'false' ]]; then
echo 'Restoring existing immutable packages from S3...'
aws s3 sync "s3://$s3_bucket/$repository_path/Packages/" "$packages_dir/" \
--exclude '*' --include '*.rpm' --only-show-errors
aws s3 sync "s3://$s3_bucket/$repository_path/keys/packages/" "$package_keys_dir/" \
--exclude '*' --include '*.asc' --only-show-errors
fi

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.

For now, this is out of scope, but later on when we want to start supporting other distros, we might want to refactor this out to another github action.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants