Skip to content

fix: ensure FEDRAMP env var is passed to operator deployment through PKO template#462

Merged
openshift-merge-bot[bot] merged 2 commits into
openshift:masterfrom
Ajpantuso:apantuso/fix_pko_fedramp
May 27, 2026
Merged

fix: ensure FEDRAMP env var is passed to operator deployment through PKO template#462
openshift-merge-bot[bot] merged 2 commits into
openshift:masterfrom
Ajpantuso:apantuso/fix_pko_fedramp

Conversation

@Ajpantuso

@Ajpantuso Ajpantuso commented May 27, 2026

Copy link
Copy Markdown
Contributor

Summary

Properly passed FEDRAMP env var through PKO template to operator deployment.

Summary by CodeRabbit

  • New Features

    • Added a configurable FedRAMP option for operator deployments with a hosted zone setting and wiring to deployment templates.
  • Tests

    • Added a FedRAMP test variant to exercise the new configuration.
  • Chores

    • Updated pre-commit guidance and pinned install instruction.
    • Improved Docker build ignore patterns.
    • Updated team ownership aliases.

@coderabbitai

coderabbitai Bot commented May 27, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: dcc53be2-bfe1-4503-be22-e1093af928f2

📥 Commits

Reviewing files that changed from the base of the PR and between 6976c2f and 558b6b2.

⛔ Files ignored due to path filters (16)
  • deploy_pko/.test-fixtures/default/Cleanup-OLM-Job.yaml is excluded by !**/.test-fixtures/**
  • deploy_pko/.test-fixtures/default/ClusterRole-certman-operator.yaml is excluded by !**/.test-fixtures/**
  • deploy_pko/.test-fixtures/default/ClusterRoleBinding-certman-operator.yaml is excluded by !**/.test-fixtures/**
  • deploy_pko/.test-fixtures/default/CustomResourceDefinition-certificaterequests.certman.managed.openshift.io.yaml is excluded by !**/.test-fixtures/**
  • deploy_pko/.test-fixtures/default/Deployment-certman-operator.yaml is excluded by !**/.test-fixtures/**
  • deploy_pko/.test-fixtures/default/Role-prometheus-k8s.yaml is excluded by !**/.test-fixtures/**
  • deploy_pko/.test-fixtures/default/RoleBinding-prometheus-k8s.yaml is excluded by !**/.test-fixtures/**
  • deploy_pko/.test-fixtures/default/ServiceAccount-certman-operator.yaml is excluded by !**/.test-fixtures/**
  • deploy_pko/.test-fixtures/fedramp/Cleanup-OLM-Job.yaml is excluded by !**/.test-fixtures/**
  • deploy_pko/.test-fixtures/fedramp/ClusterRole-certman-operator.yaml is excluded by !**/.test-fixtures/**
  • deploy_pko/.test-fixtures/fedramp/ClusterRoleBinding-certman-operator.yaml is excluded by !**/.test-fixtures/**
  • deploy_pko/.test-fixtures/fedramp/CustomResourceDefinition-certificaterequests.certman.managed.openshift.io.yaml is excluded by !**/.test-fixtures/**
  • deploy_pko/.test-fixtures/fedramp/Deployment-certman-operator.yaml is excluded by !**/.test-fixtures/**
  • deploy_pko/.test-fixtures/fedramp/Role-prometheus-k8s.yaml is excluded by !**/.test-fixtures/**
  • deploy_pko/.test-fixtures/fedramp/RoleBinding-prometheus-k8s.yaml is excluded by !**/.test-fixtures/**
  • deploy_pko/.test-fixtures/fedramp/ServiceAccount-certman-operator.yaml is excluded by !**/.test-fixtures/**
📒 Files selected for processing (4)
  • deploy_pko/.dockerignore
  • deploy_pko/Deployment-certman-operator.yaml.gotmpl
  • deploy_pko/manifest.yaml
  • hack/pko/clusterpackage.yaml
✅ Files skipped from review due to trivial changes (1)
  • deploy_pko/.dockerignore
🚧 Files skipped from review as they are similar to previous changes (2)
  • deploy_pko/Deployment-certman-operator.yaml.gotmpl
  • hack/pko/clusterpackage.yaml

Walkthrough

This PR introduces FEDRAMP as a configurable option for the certman-operator, replacing a hardcoded value with a templated configuration property. It also includes updates to pre-commit setup, team ownership in OWNERS_ALIASES, and Docker build context filtering.

Changes

FEDRAMP Configuration Feature

Layer / File(s) Summary
Schema definition and test templates
deploy_pko/manifest.yaml
Added fedramp as a string property in the OpenAPI schema with default "false", and expanded manifest test templates to include a test case that validates fedramp: "true" configuration.
Deployment and cluster package configuration
deploy_pko/Deployment-certman-operator.yaml.gotmpl, hack/pko/clusterpackage.yaml
Updated Deployment template to render FEDRAMP from {{ .config.fedramp }} instead of hardcoded 'false', added HOSTED_ZONE_ID parameter, and refactored ClusterPackage config to use dedicated fedramp and hosted_zone_id fields instead of prior env wiring.

Infrastructure and Administrative Updates

Layer / File(s) Summary
Pre-commit configuration and documentation
.pre-commit-config.yaml
Replaced generic pip installation guidance with pinned version pre-commit==4.6.0, added a uv quick-start alternative, and included a comment referencing git blame ignore-revs-file documentation.
Team ownership and build context
OWNERS_ALIASES, deploy_pko/.dockerignore
Updated srep-functional-team-aurora alias by adding BATMAN-JD and geowa4 while removing luis-falcon, and added .test-fixtures to dockerignore to exclude test fixtures from Docker build context.

🎯 2 (Simple) | ⏱️ ~10 minutes


Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error, 1 warning)

Check name Status Explanation Resolution
No-Sensitive-Data-In-Logs ❌ Error PR adds code that logs AWS Route 53 Hosted Zone ID (line 96 of certificaterequest_controller.go) - sensitive infrastructure identifier that could expose internal DNS infrastructure. Remove or redact the fedrampHostedZoneID from the logging statement on line 96, or log a generic message without the actual zone ID value.
Microshift Test Compatibility ⚠️ Warning New e2e tests use MicroShift-incompatible APIs (config.openshift.io, hive.openshift.io, operators.coreos.com) without protection labels or guards. Add [apigroup:...] tags to test names for incompatible APIs or [Skipped:MicroShift] labels, or guard tests with exutil.IsMicroShiftCluster() checks.
✅ Passed checks (13 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title clearly summarizes the main change: fixing the FEDRAMP environment variable passing through the PKO template to the operator deployment, which is the primary objective of the changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names ✅ Passed All Ginkgo test names across the repository use static strings with no dynamic content like fmt.Sprintf, variables, or string concatenation.
Test Structure And Quality ✅ Passed New e2e Ginkgo test file meets all requirements: single-responsibility tests, proper BeforeAll/AfterAll, all Eventually calls have timeouts, assertions have messages, consistent with codebase.
Single Node Openshift (Sno) Test Compatibility ✅ Passed No new Ginkgo e2e tests were added in this PR. Changes are deployment configurations, manifests, and templates only.
Topology-Aware Scheduling Compatibility ✅ Passed No topology-unaware scheduling constraints found. Deployment uses single replica with no affinity, nodeSelector, or topology constraints that would break on SNO, TNF, TNA, or HyperShift topologies.
Ote Binary Stdout Contract ✅ Passed PR only adds configuration/YAML files and new Go code; no process-level stdout writes in modifications. Test runner uses proper Ginkgo v2 setup without violating OTE stdout contract.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed PR contains no new Ginkgo e2e tests. All modified files are configuration/manifest files (YAML, templates, ignore rules) with no test code.
No-Weak-Crypto ✅ Passed PR introduces only configuration changes (YAML, Docker files, team aliases) for passing FEDRAMP environment variables; no cryptographic code or weak crypto algorithms are introduced.
Container-Privileges ✅ Passed No privileged container configurations (privileged: true, hostPID/Network/IPC, SYS_ADMIN, allowPrivilegeEscalation: true) found in any K8s manifests modified by this PR.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Ajpantuso Ajpantuso changed the title Apantuso/fix pko fedramp fix: ensure FEDRAMP env var is passed to operator deployment through PKO template May 27, 2026
@openshift-ci
openshift-ci Bot requested review from Mhodesty and ravitri May 27, 2026 16:04

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 1

🧹 Nitpick comments (1)
deploy_pko/manifest.yaml (1)

35-38: ⚡ Quick win

Constrain fedramp to valid values in the schema.

The controller only enables FedRAMP on exact "true", so a free-form string here can silently misconfigure behavior. Prefer type: boolean or a string enum ("true"|"false").

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@deploy_pko/manifest.yaml` around lines 35 - 38, The manifest's fedramp field
is currently a free-form string which can silently misconfigure the controller
(it only enables FedRAMP on exact "true"); change the schema for the fedramp
property to a constrained type—either set type: boolean with default: false, or
keep type: string and add enum: ["true","false"] and default: "false"—so the
controller receives only valid values; update the fedramp declaration (and its
default) accordingly in manifest.yaml.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@deploy_pko/Deployment-certman-operator.yaml.gotmpl`:
- Line 45: The template currently enables FedRAMP via value: '{{ .config.fedramp
}}' but does not propagate HOSTED_ZONE_ID, causing reconciles to fail when
fedramp is true; update the Deployment-certman-operator.yaml.gotmpl to set the
HOSTED_ZONE_ID environment variable from the config (e.g. value: '{{
.config.hosted_zone_id }}'), and add a hosted_zone_id config field/schema and
propagation in your package manifests so the cluster/package YAMLs expose and
pass hosted_zone_id through to the rendered template; ensure this matches the
controller expectation for HOSTED_ZONE_ID used in
certificaterequest_controller.go.

---

Nitpick comments:
In `@deploy_pko/manifest.yaml`:
- Around line 35-38: The manifest's fedramp field is currently a free-form
string which can silently misconfigure the controller (it only enables FedRAMP
on exact "true"); change the schema for the fedramp property to a constrained
type—either set type: boolean with default: false, or keep type: string and add
enum: ["true","false"] and default: "false"—so the controller receives only
valid values; update the fedramp declaration (and its default) accordingly in
manifest.yaml.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 91d823b5-573a-472c-9a40-b5c19ecd03d5

📥 Commits

Reviewing files that changed from the base of the PR and between d54b5be and 3d5e1e4.

⛔ Files ignored due to path filters (24)
  • boilerplate/_data/last-boilerplate-commit is excluded by !boilerplate/**
  • boilerplate/openshift/golang-osd-operator/OWNERS_ALIASES is excluded by !boilerplate/**
  • boilerplate/openshift/golang-osd-operator/dependabot.yml is excluded by !boilerplate/**
  • boilerplate/openshift/golang-osd-operator/docs/pre-commit.md is excluded by !boilerplate/**
  • boilerplate/openshift/golang-osd-operator/olm_pko_migration.py is excluded by !boilerplate/**
  • boilerplate/openshift/golang-osd-operator/pre-commit-config.yaml is excluded by !boilerplate/**
  • build/Dockerfile is excluded by !build/**
  • build/Dockerfile.olm-registry is excluded by !build/**
  • deploy_pko/.test-fixtures/default/Cleanup-OLM-Job.yaml is excluded by !**/.test-fixtures/**
  • deploy_pko/.test-fixtures/default/ClusterRole-certman-operator.yaml is excluded by !**/.test-fixtures/**
  • deploy_pko/.test-fixtures/default/ClusterRoleBinding-certman-operator.yaml is excluded by !**/.test-fixtures/**
  • deploy_pko/.test-fixtures/default/CustomResourceDefinition-certificaterequests.certman.managed.openshift.io.yaml is excluded by !**/.test-fixtures/**
  • deploy_pko/.test-fixtures/default/Deployment-certman-operator.yaml is excluded by !**/.test-fixtures/**
  • deploy_pko/.test-fixtures/default/Role-prometheus-k8s.yaml is excluded by !**/.test-fixtures/**
  • deploy_pko/.test-fixtures/default/RoleBinding-prometheus-k8s.yaml is excluded by !**/.test-fixtures/**
  • deploy_pko/.test-fixtures/default/ServiceAccount-certman-operator.yaml is excluded by !**/.test-fixtures/**
  • deploy_pko/.test-fixtures/fedramp/Cleanup-OLM-Job.yaml is excluded by !**/.test-fixtures/**
  • deploy_pko/.test-fixtures/fedramp/ClusterRole-certman-operator.yaml is excluded by !**/.test-fixtures/**
  • deploy_pko/.test-fixtures/fedramp/ClusterRoleBinding-certman-operator.yaml is excluded by !**/.test-fixtures/**
  • deploy_pko/.test-fixtures/fedramp/CustomResourceDefinition-certificaterequests.certman.managed.openshift.io.yaml is excluded by !**/.test-fixtures/**
  • deploy_pko/.test-fixtures/fedramp/Deployment-certman-operator.yaml is excluded by !**/.test-fixtures/**
  • deploy_pko/.test-fixtures/fedramp/Role-prometheus-k8s.yaml is excluded by !**/.test-fixtures/**
  • deploy_pko/.test-fixtures/fedramp/RoleBinding-prometheus-k8s.yaml is excluded by !**/.test-fixtures/**
  • deploy_pko/.test-fixtures/fedramp/ServiceAccount-certman-operator.yaml is excluded by !**/.test-fixtures/**
📒 Files selected for processing (6)
  • .pre-commit-config.yaml
  • OWNERS_ALIASES
  • deploy_pko/.dockerignore
  • deploy_pko/Deployment-certman-operator.yaml.gotmpl
  • deploy_pko/manifest.yaml
  • hack/pko/clusterpackage.yaml

Comment thread deploy_pko/Deployment-certman-operator.yaml.gotmpl
@codecov

codecov Bot commented May 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 57.15%. Comparing base (d54b5be) to head (558b6b2).
⚠️ Report is 4 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #462   +/-   ##
=======================================
  Coverage   57.15%   57.15%           
=======================================
  Files          29       29           
  Lines        2138     2138           
=======================================
  Hits         1222     1222           
  Misses        802      802           
  Partials      114      114           
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Ajpantuso
Ajpantuso force-pushed the apantuso/fix_pko_fedramp branch from 3d5e1e4 to 6976c2f Compare May 27, 2026 16:21
@tnierman

Copy link
Copy Markdown
Member

/retest

@Ajpantuso
Ajpantuso force-pushed the apantuso/fix_pko_fedramp branch from 6976c2f to 558b6b2 Compare May 27, 2026 16:38
@tnierman

Copy link
Copy Markdown
Member

/lgtm
/approve

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label May 27, 2026
@dustman9000

Copy link
Copy Markdown
Member

/approve

@openshift-ci

openshift-ci Bot commented May 27, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Ajpantuso, dustman9000, tnierman

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 27, 2026
@fsferraz-rh

Copy link
Copy Markdown

/retest-required

@openshift-ci

openshift-ci Bot commented May 27, 2026

Copy link
Copy Markdown
Contributor

@Ajpantuso: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@openshift-merge-bot
openshift-merge-bot Bot merged commit a29c2c9 into openshift:master May 27, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants