OPRUN-4544,OPRUN-4542: add lifecycle-controller for managing catalog lifecycle pods#1285
Open
perdasilva wants to merge 2 commits intoopenshift:mainfrom
Open
OPRUN-4544,OPRUN-4542: add lifecycle-controller for managing catalog lifecycle pods#1285perdasilva wants to merge 2 commits intoopenshift:mainfrom
perdasilva wants to merge 2 commits intoopenshift:mainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a lifecycle-controller and lifecycle-server: new CLIs and managers, a controller that watches CatalogSources and provisions per-catalog lifecycle-server workloads, a lifecycle-server that indexes/serves FBC lifecycle JSON, TLS/profile support, manifests, RBAC, build wiring, and extensive unit and E2E tests. ChangesLifecycle controller + lifecycle server feature
Sequence Diagram(s)sequenceDiagram
autonumber
participant CatalogSource as CatalogSource (CR)
participant Controller as lifecycle-controller
participant KubeAPI as Kubernetes API
participant CatalogPod as Catalog Pod
participant TLSProfile as Apiserver TLSProfile
participant TLSProv as TLSConfigProvider
participant LifecycleDep as Lifecycle Deployment (per-catalog)
participant LifecycleSvc as lifecycle-server Pod
participant Client as Client
CatalogSource->>KubeAPI: create/update CatalogSource
KubeAPI->>Controller: watch event
Controller->>KubeAPI: list/watch Pods (olm.catalogSource selector)
KubeAPI->>CatalogPod: return matching pods
Controller->>CatalogPod: inspect status, image digest, node
Controller->>KubeAPI: apply per-catalog ServiceAccount/Service/Deployment/NetworkPolicy
Controller->>KubeAPI: update shared ClusterRoleBinding subjects
TLSProfile->>Controller: notify TLS profile change (optional)
Controller->>TLSProv: UpdateProfile(newSpec)
TLSProv->>LifecycleDep: provide tls.Config with GetCertificate
LifecycleDep->>LifecycleSvc: lifecycle-server starts and serves API over TLS
Client->>LifecycleSvc: HTTPS GET /api/{version}/lifecycles/{package}
LifecycleSvc->>LifecycleSvc: lookup LifecycleIndex and respond (200/404/503)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
|
Contributor
Author
|
/hold this one should come after #1284 |
openshift-ci
Bot
added
the
do-not-merge/hold
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: perdasilva 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 |
openshift-ci
Bot
added
the
approved
perdasilva
changed the title
|
@perdasilva: This pull request references OPRUN-4544 which is a valid jira issue. This pull request references OPRUN-4542 which is a valid jira issue. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci-robot
added
the
jira/valid-reference
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
pkg/lifecycle-controller/controller.go (2)
341-358: 💤 Low valueConsider defensive validation for edge cases in
resourceName.While CatalogSource names must be valid Kubernetes object names (and thus DNS-compliant), the transformation could theoretically produce an invalid DNS label in edge cases:
- Input
"..."→"---"→""after TrimRight →"-lifecycle-server"(starts with hyphen)- Input
"123"→"123-lifecycle-server"(starts with digit, technically invalid for DNS subdomain)In practice, CatalogSource names follow Kubernetes naming rules, making these cases unlikely. However, adding a simple validation or using
strings.TrimLeft(csName, "-")after processing would be more defensive.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/lifecycle-controller/controller.go` around lines 341 - 358, Update resourceName to defensively remove leading hyphens and ensure the final label starts with a letter: after the existing TrimRight(csName, "-") call, add strings.TrimLeft(csName, "-") to remove leading dashes, and if csName is empty set csName = "a" (or otherwise ensure it will not be empty before appending the suffix). Also, if the first rune of csName is not a lowercase letter (e.g., it starts with a digit), prefix it with "a" so the final value (csName + "-" + resourceBaseName) begins with a letter; adjust truncation logic if necessary so the combined length still respects maxPrefix. Reference: function resourceName and constant resourceBaseName.
696-710: 💤 Low valueTLS profile change handler swallows list error.
When listing CatalogSources fails (line 699), the error is logged but the function returns
nil, causing no reconciliation requests to be enqueued. This silently drops the TLS profile update. Consider returning an error or implementing retry logic.Note: The current behavior may be intentional since individual CatalogSources will eventually reconcile on their own triggers, picking up the new TLS config. However, this could delay TLS profile propagation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/lifecycle-controller/controller.go` around lines 696 - 710, The handler passed to bldr.WatchesRawSource (handler.TypedEnqueueRequestsFromMapFunc) currently swallows errors from mgr.GetClient().List and returns nil, dropping the TLS profile update; change the map func to accept the full configv1.TLSProfile object (rather than TLSProfileSpec) so you can return a reconcile.Request that requeues the TLSProfile itself when mgr.GetClient().List fails (use client.ObjectKeyFromObject on the TLSProfile param), and keep the original behavior of returning CatalogSource requests on success; update the TypedEnqueueRequestsFromMapFunc signature and its callers accordingly and ensure r.Log.Error still logs the error.pkg/lifecycle-controller/controller_test.go (1)
108-163: 💤 Low valueGood test coverage for
resourceName- consider adding edge case tests.The test cases cover common scenarios well (special characters, truncation, trailing hyphens). Consider adding tests for edge cases that could produce invalid DNS labels:
- Input containing only special characters (e.g.,
"..."or"___")- Input starting with digits after transformation
These are unlikely in practice but would document the function's behavior boundaries.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/lifecycle-controller/controller_test.go` around lines 108 - 163, Add tests in TestResourceName to cover edge cases where the input is only special characters (e.g., "..." or "___") and where the transformed name starts with digits; call resourceName with these inputs and assert the returned string respects DNS label rules (lowercase, hyphens only, no leading/trailing hyphen, max 63 chars) and matches the expected output you decide (e.g., suffix-only "lifecycle-server" or a cleaned name that does not start with a digit). Use the same t.Run structure and require assertions (require.Equal and require.LessOrEqual) so TestResourceName continues to validate length and exact output for these edge-case inputs.cmd/lifecycle-controller/start.go (1)
204-209: 💤 Low valueAdd logging when falling back to default TLS profile.
When
FetchAPIServerTLSProfilefails, the function silently returns the default profile withEnableTLSProfileWatcher = false. This hides potential configuration issues. Consider logging the error to aid debugging.Proposed fix
func getInitialTLSProfile(ctx context.Context, restConfig *rest.Config, sch *runtime.Scheme) (configv1.TLSProfileSpec, bool, error) { cl, err := client.New(restConfig, client.Options{Scheme: sch}) if err != nil { return configv1.TLSProfileSpec{}, false, fmt.Errorf("failed to create client: %w", err) } initialTLSProfileSpec, err := tlsutil.FetchAPIServerTLSProfile(ctx, cl) if err != nil { + klog.V(2).Info("unable to fetch APIServer TLS profile, using default", "error", err) return *configv1.TLSProfiles[crypto.DefaultTLSProfileType], false, nil } return initialTLSProfileSpec, true, nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/lifecycle-controller/start.go` around lines 204 - 209, When tlsutil.FetchAPIServerTLSProfile(ctx, cl) returns an error, add a log statement that records the error and context before returning the default profile; specifically, log the error (e.g., using klog.Errorf or the controller's logger) inside the error branch that currently returns *configv1.TLSProfiles[crypto.DefaultTLSProfileType], false, nil so callers still get the default profile and EnableTLSProfileWatcher=false but the failure is visible for debugging.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@go.mod`:
- Line 254: The replace directive referencing
github.com/joelanford/controller-runtime-common with pseudo-version ending in
afe447e6c57e is incorrect because that commit exists only in upstream
openshift/controller-runtime-common; either change the replace to point to the
upstream module (github.com/openshift/controller-runtime-common) at the
PR/commit that contains afe447e6c57e, or update the fork
(github.com/joelanford/controller-runtime-common) to include that commit and
re-generate the pseudo-version; also add a clear comment in go.mod next to the
replace indicating this is a temporary override and include a planned removal
date or ticket reference so the replace can be removed once the correct upstream
release is available.
In `@manifests/0000_50_olm_08-lifecycle-controller.deployment.yaml`:
- Around line 62-67: The GOMEMLIMIT env var is set to an unreasonably low "5MiB"
causing excessive GC; update the GOMEMLIMIT value (env name GOMEMLIMIT) to a
realistic budget aligned with the controller pod memory (e.g., match
resources.requests.memory and resources.limits.memory — raise requests.memory
from 10Mi to a sensible value such as 128Mi and set a corresponding
resources.limits.memory), and apply the same GOMEMLIMIT and resource changes to
the mirrored microshift deployment manifests so both deployments use the same
memory budget.
In `@manifests/0000_50_olm_08-lifecycle-controller.networkpolicy.yaml`:
- Around line 20-32: The NetworkPolicy egress currently only restricts ports
(egress block listing ports 6443, 53, 5353) which allows traffic to any
destination; update the egress rules to include explicit "to" selectors for each
destination: add a rule targeting the API server endpoint (e.g., the cluster API
server IP or service via an ipBlock or a namespaceSelector/podSelector for
kube-system/kube-apiserver) for port 6443, and rules targeting the cluster DNS
service (kube-dns or coredns Service IP or selector) for ports 53 and 5353 (both
TCP/UDP as appropriate), ensuring each egress entry pairs its ports with the
correct "to" clause to enforce least privilege.
In `@microshift-manifests/0000_50_olm_08-lifecycle-controller.networkpolicy.yaml`:
- Around line 20-32: The egress rules currently allow wide-open access to ports
6443, 53 and 5353; constrain them by adding "to" destination selectors for each
port group: for the API server egress (port 6443) add a "to" that targets the
API server endpoints (e.g., podSelector or namespaceSelector that matches the
control-plane/apiserver pods or the API server Service endpoints) and for DNS
egress (ports 53 and 5353) add a "to" that targets DNS backends (e.g.,
podSelector matching app: coredns or k8s-app: kube-dns or the cluster DNS
Service endpoints); update the egress entries around the port lists (ports: 6443
/ ports: 53,5353) to include the corresponding "to" blocks so traffic is limited
to the API server and DNS targets only.
In `@scripts/generate_crds_manifests.sh`:
- Around line 763-766: The RBAC block that manages lifecycle-server
clusterrolebindings currently includes the "delete" verb for resource
"clusterrolebindings"; remove "delete" from the verbs list (leave "get", "list",
"watch", "create", "update", "patch") so the reconciler can manage/apply CRBs
but cannot delete them, tightening permissions for the lifecycle-server
clusterrolebindings entry that references "clusterrolebindings".
---
Nitpick comments:
In `@cmd/lifecycle-controller/start.go`:
- Around line 204-209: When tlsutil.FetchAPIServerTLSProfile(ctx, cl) returns an
error, add a log statement that records the error and context before returning
the default profile; specifically, log the error (e.g., using klog.Errorf or the
controller's logger) inside the error branch that currently returns
*configv1.TLSProfiles[crypto.DefaultTLSProfileType], false, nil so callers still
get the default profile and EnableTLSProfileWatcher=false but the failure is
visible for debugging.
In `@pkg/lifecycle-controller/controller_test.go`:
- Around line 108-163: Add tests in TestResourceName to cover edge cases where
the input is only special characters (e.g., "..." or "___") and where the
transformed name starts with digits; call resourceName with these inputs and
assert the returned string respects DNS label rules (lowercase, hyphens only, no
leading/trailing hyphen, max 63 chars) and matches the expected output you
decide (e.g., suffix-only "lifecycle-server" or a cleaned name that does not
start with a digit). Use the same t.Run structure and require assertions
(require.Equal and require.LessOrEqual) so TestResourceName continues to
validate length and exact output for these edge-case inputs.
In `@pkg/lifecycle-controller/controller.go`:
- Around line 341-358: Update resourceName to defensively remove leading hyphens
and ensure the final label starts with a letter: after the existing
TrimRight(csName, "-") call, add strings.TrimLeft(csName, "-") to remove leading
dashes, and if csName is empty set csName = "a" (or otherwise ensure it will not
be empty before appending the suffix). Also, if the first rune of csName is not
a lowercase letter (e.g., it starts with a digit), prefix it with "a" so the
final value (csName + "-" + resourceBaseName) begins with a letter; adjust
truncation logic if necessary so the combined length still respects maxPrefix.
Reference: function resourceName and constant resourceBaseName.
- Around line 696-710: The handler passed to bldr.WatchesRawSource
(handler.TypedEnqueueRequestsFromMapFunc) currently swallows errors from
mgr.GetClient().List and returns nil, dropping the TLS profile update; change
the map func to accept the full configv1.TLSProfile object (rather than
TLSProfileSpec) so you can return a reconcile.Request that requeues the
TLSProfile itself when mgr.GetClient().List fails (use
client.ObjectKeyFromObject on the TLSProfile param), and keep the original
behavior of returning CatalogSource requests on success; update the
TypedEnqueueRequestsFromMapFunc signature and its callers accordingly and ensure
r.Log.Error still logs the error.
🪄 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: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: b1015d3d-9bae-4419-9199-6f68e8e99b31
⛔ Files ignored due to path filters (6)
go.sumis excluded by!**/*.sumvendor/github.com/openshift/controller-runtime-common/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/controller-runtime-common/pkg/tls/controller.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/controller-runtime-common/pkg/tls/tls.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/library-go/pkg/crypto/crypto.gois excluded by!**/vendor/**,!vendor/**vendor/modules.txtis excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (29)
Makefilecmd/lifecycle-controller/main.gocmd/lifecycle-controller/start.gocmd/lifecycle-controller/util.gocmd/lifecycle-server/main.gocmd/lifecycle-server/start.gogo.modmanifests/0000_50_olm_08-lifecycle-controller.deployment.ibm-cloud-managed.yamlmanifests/0000_50_olm_08-lifecycle-controller.deployment.yamlmanifests/0000_50_olm_08-lifecycle-controller.networkpolicy.yamlmanifests/0000_50_olm_08-lifecycle-controller.rbac.yamlmanifests/0000_50_olm_08-lifecycle-controller.service.yamlmicroshift-manifests/0000_50_olm_08-lifecycle-controller.deployment.ibm-cloud-managed.yamlmicroshift-manifests/0000_50_olm_08-lifecycle-controller.deployment.yamlmicroshift-manifests/0000_50_olm_08-lifecycle-controller.networkpolicy.yamlmicroshift-manifests/0000_50_olm_08-lifecycle-controller.rbac.yamlmicroshift-manifests/0000_50_olm_08-lifecycle-controller.service.yamlmicroshift-manifests/kustomization.yamloperator-lifecycle-manager.Dockerfilepkg/lifecycle-controller/TODO.mdpkg/lifecycle-controller/controller.gopkg/lifecycle-controller/controller_test.gopkg/lifecycle-controller/tls.gopkg/lifecycle-controller/tls_test.gopkg/lifecycle-server/fbc.gopkg/lifecycle-server/fbc_test.gopkg/lifecycle-server/server.gopkg/lifecycle-server/server_test.goscripts/generate_crds_manifests.sh
perdasilva
force-pushed
the
lifecycle-controller
branch
3 times, most recently
from
74f8725 to
c148b33
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
go.mod (1)
254-254:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winThe replace directive issue flagged in previous review remains unresolved.
As noted in the prior review, this replace directive points to a fork (
joelanford/controller-runtime-common) that does not contain commitafe447e6c57e. The commit exists only in the upstreamopenshift/controller-runtime-commonrepository. This mismatch will cause module resolution failures.Please address the previous review comment by either:
- Updating the replace to point to the upstream repository, or
- Ensuring the fork includes the required commit
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@go.mod` at line 254, The replace directive currently mapping github.com/openshift/controller-runtime-common to github.com/joelanford/controller-runtime-common at pseudo-version afe447e6c57e is invalid because that fork does not contain that commit; update the go.mod replace so the module resolution points to the upstream repo or to a fork that actually contains commit afe447e6c57e — specifically change the replace target from github.com/joelanford/controller-runtime-common to github.com/openshift/controller-runtime-common (or ensure the joelanford fork is updated to include commit afe447e6c57e) so the existing replace line and pseudo-version resolve correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/lifecycle-controller/start.go`:
- Around line 204-206: The current code in start.go masks all errors from
tlsutil.FetchAPIServerTLSProfile by unconditionally falling back to the default
profile and disabling the watcher; change the error handling so only the
specific "not found / no TLS profile configured" condition falls back: call
tlsutil.FetchAPIServerTLSProfile(ctx, cl) and if the returned error is the
sentinel/not-found condition (e.g., apierrors.IsNotFound(err) or a
tlsutil.ErrNoProfile sentinel exposed by tlsutil) then return
*configv1.TLSProfiles[crypto.DefaultTLSProfileType], false, nil; for any other
error from FetchAPIServerTLSProfile return nil (or propagate the error) so
startup fails and the watcher remains active; update the branch around
initialTLSProfileSpec and its error handling accordingly.
- Around line 278-300: The TLS watcher currently sends events into an unbuffered
tlsChangeChan in setupTLSProfileWatcher which can block the OnProfileChange
callback; change tlsChangeChan to a buffered channel (e.g., make(chan
event.TypedGenericEvent[configv1.TLSProfileSpec], 1) or a small configurable
buffer) so sends in tlsChangeChan <- ... inside the OnProfileChange closure
never block the watcher or shutdown path; update any callers/consumers if they
rely on synchronous behavior.
In `@pkg/lifecycle-controller/controller.go`:
- Around line 532-537: The egress rule added via WithEgress /
networkingv1ac.NetworkPolicyEgressRule currently only allows the backend
targetPort 6443 (specified with NetworkPolicyPort and intstr.FromInt32(6443)),
but Kubernetes services are reached via their service port (443) so
TokenReview/SAR calls will be blocked; update the NetworkPolicyPort list in the
WithEgress block (where NetworkPolicyEgressRule is built) to include a second
port entry for 443 in addition to 6443 so the policy permits traffic to
kubernetes.default.svc:443.
---
Duplicate comments:
In `@go.mod`:
- Line 254: The replace directive currently mapping
github.com/openshift/controller-runtime-common to
github.com/joelanford/controller-runtime-common at pseudo-version afe447e6c57e
is invalid because that fork does not contain that commit; update the go.mod
replace so the module resolution points to the upstream repo or to a fork that
actually contains commit afe447e6c57e — specifically change the replace target
from github.com/joelanford/controller-runtime-common to
github.com/openshift/controller-runtime-common (or ensure the joelanford fork is
updated to include commit afe447e6c57e) so the existing replace line and
pseudo-version resolve correctly.
🪄 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: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: e97221ef-a124-4bdf-b6d8-66af5a489b65
⛔ Files ignored due to path filters (5)
go.sumis excluded by!**/*.sumvendor/github.com/openshift/controller-runtime-common/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/controller-runtime-common/pkg/tls/controller.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/controller-runtime-common/pkg/tls/tls.gois excluded by!**/vendor/**,!vendor/**vendor/modules.txtis excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (25)
Makefilecmd/lifecycle-controller/main.gocmd/lifecycle-controller/start.gocmd/lifecycle-controller/util.gogo.modmanifests/0000_50_olm_08-lifecycle-controller.deployment.ibm-cloud-managed.yamlmanifests/0000_50_olm_08-lifecycle-controller.deployment.yamlmanifests/0000_50_olm_08-lifecycle-controller.networkpolicy.yamlmanifests/0000_50_olm_08-lifecycle-controller.rbac.yamlmanifests/0000_50_olm_08-lifecycle-controller.service.yamlmanifests/0000_50_olm_09-lifecycle-server.rbac.yamlmicroshift-manifests/0000_50_olm_08-lifecycle-controller.deployment.ibm-cloud-managed.yamlmicroshift-manifests/0000_50_olm_08-lifecycle-controller.deployment.yamlmicroshift-manifests/0000_50_olm_08-lifecycle-controller.networkpolicy.yamlmicroshift-manifests/0000_50_olm_08-lifecycle-controller.rbac.yamlmicroshift-manifests/0000_50_olm_08-lifecycle-controller.service.yamlmicroshift-manifests/0000_50_olm_09-lifecycle-server.rbac.yamlmicroshift-manifests/kustomization.yamloperator-lifecycle-manager.Dockerfilepkg/lifecycle-controller/TODO.mdpkg/lifecycle-controller/controller.gopkg/lifecycle-controller/controller_test.gopkg/lifecycle-controller/tls.gopkg/lifecycle-controller/tls_test.goscripts/generate_crds_manifests.sh
✅ Files skipped from review due to trivial changes (10)
- microshift-manifests/0000_50_olm_09-lifecycle-server.rbac.yaml
- pkg/lifecycle-controller/TODO.md
- manifests/0000_50_olm_09-lifecycle-server.rbac.yaml
- operator-lifecycle-manager.Dockerfile
- microshift-manifests/0000_50_olm_08-lifecycle-controller.networkpolicy.yaml
- manifests/0000_50_olm_08-lifecycle-controller.networkpolicy.yaml
- manifests/0000_50_olm_08-lifecycle-controller.service.yaml
- microshift-manifests/0000_50_olm_08-lifecycle-controller.rbac.yaml
- pkg/lifecycle-controller/tls_test.go
- scripts/generate_crds_manifests.sh
🚧 Files skipped from review as they are similar to previous changes (5)
- microshift-manifests/kustomization.yaml
- microshift-manifests/0000_50_olm_08-lifecycle-controller.service.yaml
- cmd/lifecycle-controller/util.go
- cmd/lifecycle-controller/main.go
- manifests/0000_50_olm_08-lifecycle-controller.deployment.yaml
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 895ba5ca-03fc-4a12-890d-41282603cecd
⛔ Files ignored due to path filters (27)
go.sumis excluded by!**/*.sumvendor/github.com/openshift/api/config/v1/types_apiserver.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_authentication.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_cluster_version.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_ingress.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_insights.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_network.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_tlssecurityprofile.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1alpha1/register.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/types_cluster_image_policy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/types_cluster_monitoring.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/types_crio_credential_provider_config.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/types_image_policy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/types_insights.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/types_pki.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1alpha1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1alpha1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1alpha2/types_insights.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha2/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/controller-runtime-common/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/controller-runtime-common/pkg/tls/controller.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/controller-runtime-common/pkg/tls/tls.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/library-go/pkg/crypto/crypto.gois excluded by!**/vendor/**,!vendor/**vendor/modules.txtis excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (25)
Makefilecmd/lifecycle-controller/main.gocmd/lifecycle-controller/start.gocmd/lifecycle-controller/util.gogo.modmanifests/0000_50_olm_08-lifecycle-controller.deployment.ibm-cloud-managed.yamlmanifests/0000_50_olm_08-lifecycle-controller.deployment.yamlmanifests/0000_50_olm_08-lifecycle-controller.networkpolicy.yamlmanifests/0000_50_olm_08-lifecycle-controller.rbac.yamlmanifests/0000_50_olm_08-lifecycle-controller.service.yamlmanifests/0000_50_olm_09-lifecycle-server.rbac.yamlmicroshift-manifests/0000_50_olm_08-lifecycle-controller.deployment.ibm-cloud-managed.yamlmicroshift-manifests/0000_50_olm_08-lifecycle-controller.deployment.yamlmicroshift-manifests/0000_50_olm_08-lifecycle-controller.networkpolicy.yamlmicroshift-manifests/0000_50_olm_08-lifecycle-controller.rbac.yamlmicroshift-manifests/0000_50_olm_08-lifecycle-controller.service.yamlmicroshift-manifests/0000_50_olm_09-lifecycle-server.rbac.yamlmicroshift-manifests/kustomization.yamloperator-lifecycle-manager.Dockerfilepkg/lifecycle-controller/TODO.mdpkg/lifecycle-controller/controller.gopkg/lifecycle-controller/controller_test.gopkg/lifecycle-controller/tls.gopkg/lifecycle-controller/tls_test.goscripts/generate_crds_manifests.sh
✅ Files skipped from review due to trivial changes (12)
- microshift-manifests/kustomization.yaml
- operator-lifecycle-manager.Dockerfile
- microshift-manifests/0000_50_olm_08-lifecycle-controller.service.yaml
- pkg/lifecycle-controller/TODO.md
- microshift-manifests/0000_50_olm_08-lifecycle-controller.networkpolicy.yaml
- manifests/0000_50_olm_08-lifecycle-controller.networkpolicy.yaml
- pkg/lifecycle-controller/tls_test.go
- manifests/0000_50_olm_08-lifecycle-controller.service.yaml
- cmd/lifecycle-controller/util.go
- microshift-manifests/0000_50_olm_08-lifecycle-controller.rbac.yaml
- manifests/0000_50_olm_08-lifecycle-controller.deployment.yaml
- scripts/generate_crds_manifests.sh
🚧 Files skipped from review as they are similar to previous changes (5)
- manifests/0000_50_olm_09-lifecycle-server.rbac.yaml
- microshift-manifests/0000_50_olm_09-lifecycle-server.rbac.yaml
- microshift-manifests/0000_50_olm_08-lifecycle-controller.deployment.ibm-cloud-managed.yaml
- go.mod
- microshift-manifests/0000_50_olm_08-lifecycle-controller.deployment.yaml
perdasilva
force-pushed
the
lifecycle-controller
branch
2 times, most recently
from
7354ff8 to
0299300
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
staging/operator-lifecycle-manager/test/images/downstream-lifecycle-controller/lifecycle-catalog/Dockerfile (1)
1-2: ⚡ Quick winPrefer
COPYhere.
ADDis unnecessary for a local config directory and carries tar/URL semantics we don't need in this fixture.Suggested fix
FROM scratch -ADD configs /configs +COPY configs /configs🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@staging/operator-lifecycle-manager/test/images/downstream-lifecycle-controller/lifecycle-catalog/Dockerfile` around lines 1 - 2, The Dockerfile uses "ADD configs /configs" which is unnecessary; replace that ADD instruction with "COPY configs /configs" so the local configs directory is copied without ADD's tar/URL behavior; update the Dockerfile line that currently contains ADD configs /configs to use COPY instead and keep the same destination path.staging/operator-lifecycle-manager/test/images/downstream-lifecycle-controller/lifecycle-catalog-no-lifecycle/Dockerfile (1)
1-2: ⚡ Quick winPrefer
COPYhere.
ADDis unnecessary for a local config directory and carries tar/URL semantics we don't need in this fixture.Suggested fix
FROM scratch -ADD configs /configs +COPY configs /configs🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@staging/operator-lifecycle-manager/test/images/downstream-lifecycle-controller/lifecycle-catalog-no-lifecycle/Dockerfile` around lines 1 - 2, Replace the ADD instruction with COPY in the Dockerfile so the local configs directory is copied without ADD's extra tar/URL semantics; update the Dockerfile's second line to use COPY configs /configs (leave the FROM scratch line unchanged) to ensure the fixture uses the simpler, more appropriate Dockerfile directive.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@microshift-manifests/0000_50_olm_08-lifecycle-controller.deployment.yaml`:
- Line 48: The startup argument
--catalog-source-field-selector=metadata.namespace=openshift-marketplace
hard-codes a namespace filter and prevents the lifecycle-controller from seeing
CatalogSource objects outside openshift-marketplace; remove this argument (or
replace it with a configurable value passed via an environment variable or
downward API) from the lifecycle-controller container args so the controller can
observe CatalogSources across namespaces (look for the lifecycle-controller
Deployment args list containing --catalog-source-field-selector and update it
accordingly).
In `@pkg/lifecycle-controller/controller.go`:
- Around line 151-166: The current loop returns the first running pod matching
the label which is non-deterministic; instead, filter pods to those with
Phase==corev1.PodRunning and with a Ready condition true, then choose a
deterministic candidate (for example pick the pod with the most recent
StartTime/CreationTimestamp) and return its imageID(p) and Spec.NodeName; update
the code around r.List and the loop that uses pods.Items to build a slice of
ready running pods, sort/select by pod.Status.StartTime (or CreationTimestamp)
and then call imageID(selectedPod) to return the digest and node.
- Around line 130-132: When imageRef == "" in the reconcile path (the block
around imageRef check in controller.go), instead of immediately returning,
delete any stale lifecycle-server resources (Deployment, Service,
ServiceAccount, NetworkPolicy) and remove or update the shared
ClusterRoleBinding (CRB) subject so RBAC is cleaned up, then re-sync the shared
CRB state before returning; implement or call a helper like
ensureLifecycleServerAbsent/cleanupLifecycleServerResources and a
resyncSharedCRB function from the reconcile loop (referencing imageRef,
reconcile/Reconcile method, and the ClusterRoleBinding subject handling) and
only then return ctrl.Result{}, nil (or requeue if needed).
In
`@staging/operator-lifecycle-manager/test/e2e/downstream_lifecycle_controller_test.go`:
- Around line 224-230: The Eventually block currently treats any error from
KubernetesInterface().RbacV1().ClusterRoleBindings().Get as success by returning
true; change the logic so GET errors do not satisfy the assertion: if Get
returns a NotFound error then return true (binding gone), if Get returns any
other error return false to keep retrying, and only return true when
crbContainsSubject(crb, name, ns.Name) is false; update the anonymous func in
the Eventually call that uses lcCRBName and crbContainsSubject accordingly.
---
Nitpick comments:
In
`@staging/operator-lifecycle-manager/test/images/downstream-lifecycle-controller/lifecycle-catalog-no-lifecycle/Dockerfile`:
- Around line 1-2: Replace the ADD instruction with COPY in the Dockerfile so
the local configs directory is copied without ADD's extra tar/URL semantics;
update the Dockerfile's second line to use COPY configs /configs (leave the FROM
scratch line unchanged) to ensure the fixture uses the simpler, more appropriate
Dockerfile directive.
In
`@staging/operator-lifecycle-manager/test/images/downstream-lifecycle-controller/lifecycle-catalog/Dockerfile`:
- Around line 1-2: The Dockerfile uses "ADD configs /configs" which is
unnecessary; replace that ADD instruction with "COPY configs /configs" so the
local configs directory is copied without ADD's tar/URL behavior; update the
Dockerfile line that currently contains ADD configs /configs to use COPY instead
and keep the same destination path.
🪄 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: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: ed000ad1-d469-4ecf-b22b-bb6685a2913c
⛔ Files ignored due to path filters (27)
go.sumis excluded by!**/*.sumvendor/github.com/openshift/api/config/v1/types_apiserver.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_authentication.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_cluster_version.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_ingress.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_insights.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_network.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_tlssecurityprofile.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1alpha1/register.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/types_cluster_image_policy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/types_cluster_monitoring.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/types_crio_credential_provider_config.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/types_image_policy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/types_insights.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/types_pki.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1alpha1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1alpha1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1alpha2/types_insights.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha2/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/controller-runtime-common/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/controller-runtime-common/pkg/tls/controller.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/controller-runtime-common/pkg/tls/tls.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/library-go/pkg/crypto/crypto.gois excluded by!**/vendor/**,!vendor/**vendor/modules.txtis excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (32)
Makefilecmd/lifecycle-controller/main.gocmd/lifecycle-controller/start.gocmd/lifecycle-controller/util.gogo.modmanifests/0000_50_olm_08-lifecycle-controller.deployment.ibm-cloud-managed.yamlmanifests/0000_50_olm_08-lifecycle-controller.deployment.yamlmanifests/0000_50_olm_08-lifecycle-controller.networkpolicy.yamlmanifests/0000_50_olm_08-lifecycle-controller.rbac.yamlmanifests/0000_50_olm_08-lifecycle-controller.service.yamlmanifests/0000_50_olm_09-lifecycle-server.rbac.yamlmicroshift-manifests/0000_50_olm_08-lifecycle-controller.deployment.ibm-cloud-managed.yamlmicroshift-manifests/0000_50_olm_08-lifecycle-controller.deployment.yamlmicroshift-manifests/0000_50_olm_08-lifecycle-controller.networkpolicy.yamlmicroshift-manifests/0000_50_olm_08-lifecycle-controller.rbac.yamlmicroshift-manifests/0000_50_olm_08-lifecycle-controller.service.yamlmicroshift-manifests/0000_50_olm_09-lifecycle-server.rbac.yamlmicroshift-manifests/kustomization.yamloperator-lifecycle-manager.Dockerfilepkg/lifecycle-controller/TODO.mdpkg/lifecycle-controller/controller.gopkg/lifecycle-controller/controller_test.gopkg/lifecycle-controller/tls.gopkg/lifecycle-controller/tls_test.goscripts/generate_crds_manifests.shstaging/operator-lifecycle-manager/test/e2e/downstream_lifecycle_controller_test.gostaging/operator-lifecycle-manager/test/e2e/downstream_lifecycle_server_test.gostaging/operator-lifecycle-manager/test/images/downstream-lifecycle-controller/build.shstaging/operator-lifecycle-manager/test/images/downstream-lifecycle-controller/lifecycle-catalog-no-lifecycle/Dockerfilestaging/operator-lifecycle-manager/test/images/downstream-lifecycle-controller/lifecycle-catalog-no-lifecycle/configs/catalog.jsonstaging/operator-lifecycle-manager/test/images/downstream-lifecycle-controller/lifecycle-catalog/Dockerfilestaging/operator-lifecycle-manager/test/images/downstream-lifecycle-controller/lifecycle-catalog/configs/catalog.json
✅ Files skipped from review due to trivial changes (13)
- microshift-manifests/kustomization.yaml
- manifests/0000_50_olm_08-lifecycle-controller.networkpolicy.yaml
- manifests/0000_50_olm_08-lifecycle-controller.service.yaml
- pkg/lifecycle-controller/TODO.md
- staging/operator-lifecycle-manager/test/images/downstream-lifecycle-controller/build.sh
- microshift-manifests/0000_50_olm_08-lifecycle-controller.service.yaml
- manifests/0000_50_olm_09-lifecycle-server.rbac.yaml
- microshift-manifests/0000_50_olm_08-lifecycle-controller.rbac.yaml
- pkg/lifecycle-controller/tls_test.go
- microshift-manifests/0000_50_olm_08-lifecycle-controller.deployment.ibm-cloud-managed.yaml
- manifests/0000_50_olm_08-lifecycle-controller.deployment.ibm-cloud-managed.yaml
- microshift-manifests/0000_50_olm_08-lifecycle-controller.networkpolicy.yaml
- cmd/lifecycle-controller/start.go
🚧 Files skipped from review as they are similar to previous changes (7)
- operator-lifecycle-manager.Dockerfile
- microshift-manifests/0000_50_olm_09-lifecycle-server.rbac.yaml
- cmd/lifecycle-controller/util.go
- cmd/lifecycle-controller/main.go
- Makefile
- manifests/0000_50_olm_08-lifecycle-controller.deployment.yaml
- scripts/generate_crds_manifests.sh
perdasilva
force-pushed
the
lifecycle-controller
branch
from
0299300 to
ef696c5
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
staging/operator-lifecycle-manager/test/images/downstream-lifecycle-controller/lifecycle-catalog-no-lifecycle/Dockerfile (1)
2-2: UseCOPYinstead ofADDfor the plain directory copy.The
COPYinstruction is the appropriate choice for copying local directories. TheADDinstruction should be reserved for remote URLs and automatic tar extraction, avoiding unintended behavior.Suggested patch
-ADD configs /configs +COPY configs /configs🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@staging/operator-lifecycle-manager/test/images/downstream-lifecycle-controller/lifecycle-catalog-no-lifecycle/Dockerfile` at line 2, The Dockerfile uses the ADD instruction to copy a local directory (ADD configs /configs); replace that with the COPY instruction to avoid unintended behavior—change the ADD configs /configs line to COPY configs /configs in the Dockerfile so the local directory is copied plainly (no automatic tar extraction or URL handling).staging/operator-lifecycle-manager/test/images/downstream-lifecycle-controller/lifecycle-catalog/Dockerfile (1)
2-2: UseCOPYinstead ofADDfor plain directory copy.
ADDhas extra semantics (URL/tar handling) that are unnecessary here and can be surprising. Sinceconfigsis a directory,COPYis the more appropriate choice.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@staging/operator-lifecycle-manager/test/images/downstream-lifecycle-controller/lifecycle-catalog/Dockerfile` at line 2, Replace the Dockerfile ADD instruction "ADD configs /configs" with the equivalent COPY instruction by changing it to "COPY configs /configs" so the plain directory copy uses COPY instead of ADD; update the Dockerfile line containing ADD configs /configs accordingly.pkg/lifecycle-controller/controller_test.go (1)
675-688: ⚡ Quick winAdd regression coverage for stale cleanup and multi-pod selection
Current tests don’t cover two critical edge paths:
- pre-existing lifecycle resources when Line 130 returns with no imageRef, and
- multiple running catalog pods requiring deterministic selection.
Adding these cases will lock in behavior and prevent regressions once controller fixes land.
Also applies to: 867-930
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/lifecycle-controller/controller_test.go` around lines 675 - 688, Add two tests to controller_test.go using the existing helpers (newCatalogSource, testClientBuilder, testReconciler) that call r.Reconcile(ctx, ctrl.Request{...}) like TestReconcile_NoPodRunning: (1) create a CatalogSource with no imageRef and pre-existing lifecycle resources (Lifecycle objects/conditions) and assert that after Reconcile those stale lifecycle resources are removed/cleaned up and Reconcile returns ctrl.Result{} with no error; (2) create a CatalogSource with multiple running catalog Pods and assert the controller's deterministic selection behavior by seeding Pods with distinct creationTimestamps/labels and verifying Reconcile selected the expected pod (e.g., earliest CreationTimestamp or the selection criterion implemented in Reconcile) and produced the expected lifecycle update; use the same test helpers and assert on object presence/absence and specific lifecycle status updates to lock in behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@manifests/0000_50_olm_08-lifecycle-controller.rbac.yaml`:
- Around line 39-55: The RBAC rules currently grant the overly broad "update"
verb for the lifecycle controller on resources "services", "serviceaccounts",
"networkpolicies", and "clusterrolebindings"; remove "update" from the verbs
arrays for the rules that target these resources (the entries that list
resources: ["services"], ["serviceaccounts"], ["networkpolicies"], and
["clusterrolebindings"]) so they only use the needed verbs (e.g.,
"get","list","watch","create","patch","delete") to enforce least privilege
without changing reconcile behavior.
In `@pkg/lifecycle-controller/controller.go`:
- Around line 341-358: resourceName currently truncates long csName directly
which can make distinct CatalogSource names collide; update resourceName to
preserve uniqueness by computing a short deterministic hash when truncation is
needed: normalize csName as now, compute suffix using resourceBaseName,
determine maxPrefix for the prefix part, and if len(csName) > maxPrefix then
compute a compact hash (e.g., sha256 and hex-encode first 6 chars), trim csName
to fit maxPrefix minus len("-")+len(hash6) so you can insert "-<hash6>" before
"-"+resourceBaseName, then trim trailing "-" and return csName + "-" + hash6 +
"-" + resourceBaseName; ensure all name length checks still enforce the 63-char
limit and keep references to the resourceName function and resourceBaseName
symbol for locating the change.
In
`@staging/operator-lifecycle-manager/test/e2e/downstream_lifecycle_server_test.go`:
- Around line 31-33: The test builds an HTTP fetch command with wget flags into
the args slice which currently includes "-S" (in the args variable assembled
with extraArgs and url), and "-S" can inject response headers into stderr that
pollutes captured logs and breaks json.Unmarshal assertions; remove the "-S"
flag from the args construction (and the other identical occurrence around lines
176-179) so args becomes []string{"-O", "/dev/stdout", "-q"} (and preserve
appending extraArgs and url), updating any test helper that assembles wget args
accordingly (locate the code that sets args, the extraArgs append sites, and the
uses that parse body to ensure they now receive clean JSON).
---
Nitpick comments:
In `@pkg/lifecycle-controller/controller_test.go`:
- Around line 675-688: Add two tests to controller_test.go using the existing
helpers (newCatalogSource, testClientBuilder, testReconciler) that call
r.Reconcile(ctx, ctrl.Request{...}) like TestReconcile_NoPodRunning: (1) create
a CatalogSource with no imageRef and pre-existing lifecycle resources (Lifecycle
objects/conditions) and assert that after Reconcile those stale lifecycle
resources are removed/cleaned up and Reconcile returns ctrl.Result{} with no
error; (2) create a CatalogSource with multiple running catalog Pods and assert
the controller's deterministic selection behavior by seeding Pods with distinct
creationTimestamps/labels and verifying Reconcile selected the expected pod
(e.g., earliest CreationTimestamp or the selection criterion implemented in
Reconcile) and produced the expected lifecycle update; use the same test helpers
and assert on object presence/absence and specific lifecycle status updates to
lock in behavior.
In
`@staging/operator-lifecycle-manager/test/images/downstream-lifecycle-controller/lifecycle-catalog-no-lifecycle/Dockerfile`:
- Line 2: The Dockerfile uses the ADD instruction to copy a local directory (ADD
configs /configs); replace that with the COPY instruction to avoid unintended
behavior—change the ADD configs /configs line to COPY configs /configs in the
Dockerfile so the local directory is copied plainly (no automatic tar extraction
or URL handling).
In
`@staging/operator-lifecycle-manager/test/images/downstream-lifecycle-controller/lifecycle-catalog/Dockerfile`:
- Line 2: Replace the Dockerfile ADD instruction "ADD configs /configs" with the
equivalent COPY instruction by changing it to "COPY configs /configs" so the
plain directory copy uses COPY instead of ADD; update the Dockerfile line
containing ADD configs /configs accordingly.
🪄 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: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 7a9ff271-0670-4015-a8af-f3a500641768
⛔ Files ignored due to path filters (262)
go.sumis excluded by!**/*.sumvendor/github.com/openshift/api/config/v1/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_apiserver.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_authentication.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_cluster_version.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_dns.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_infrastructure.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_ingress.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_insights.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_kmsencryption.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_network.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_tlssecurityprofile.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1alpha1/register.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/types_cluster_image_policy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/types_cluster_monitoring.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/types_crio_credential_provider_config.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/types_image_policy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/types_insights.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/types_pki.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1alpha1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1alpha1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1alpha2/types_insights.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha2/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/acceptrisk.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/alibabacloudplatformstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/alibabacloudresourcetag.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/apiserver.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/apiserverencryption.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/apiservernamedservingcert.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/apiserverservingcerts.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/apiserverspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/audit.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/auditcustomrule.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/authentication.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/authenticationspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/authenticationstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/awsdnsspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/awsingressspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/awskmsconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/awsplatformspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/awsplatformstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/awsresourcetag.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/awsserviceendpoint.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/azureplatformstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/azureresourcetag.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/baremetalplatformloadbalancer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/baremetalplatformspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/baremetalplatformstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/basicauthidentityprovider.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/build.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/builddefaults.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/buildoverrides.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/buildspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/cloudcontrollermanagerstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/cloudloadbalancerconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/cloudloadbalancerips.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/clustercondition.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/clusterimagepolicy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/clusterimagepolicyspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/clusterimagepolicystatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/clusternetworkentry.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/clusteroperator.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/clusteroperatorstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/clusteroperatorstatuscondition.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/clusterversion.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/clusterversioncapabilitiesspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/clusterversioncapabilitiesstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/clusterversionspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/clusterversionstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/componentoverride.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/componentroutespec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/componentroutestatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/conditionalupdate.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/conditionalupdaterisk.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/configmapfilereference.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/configmapnamereference.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/console.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/consoleauthentication.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/consolespec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/consolestatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/custom.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/customfeaturegates.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/customtlsprofile.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/deprecatedwebhooktokenauthenticator.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/dns.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/dnsplatformspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/dnsspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/dnszone.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/equinixmetalplatformstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/externalipconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/externalippolicy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/externalplatformspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/externalplatformstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/extramapping.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/featuregate.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/featuregateattributes.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/featuregatedetails.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/featuregateselection.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/featuregatestatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/gatherconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/gathererconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/gatherers.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/gcpplatformstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/gcpresourcelabel.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/gcpresourcetag.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/githubidentityprovider.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/gitlabidentityprovider.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/googleidentityprovider.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/htpasswdidentityprovider.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/hubsource.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/hubsourcestatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/ibmcloudplatformspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/ibmcloudplatformstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/ibmcloudserviceendpoint.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/identityprovider.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/identityproviderconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/image.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagecontentpolicy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagecontentpolicyspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagedigestmirrors.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagedigestmirrorset.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagedigestmirrorsetspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagelabel.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagepolicy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagepolicyfulciocawithrekorrootoftrust.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagepolicypkirootoftrust.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagepolicypublickeyrootoftrust.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagepolicyspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagepolicystatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagesigstoreverificationpolicy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagespec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagestatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagetagmirrors.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagetagmirrorset.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagetagmirrorsetspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/infrastructure.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/infrastructurespec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/infrastructurestatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/ingress.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/ingressplatformspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/ingressspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/ingressstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/insightsdatagather.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/insightsdatagatherspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/keystoneidentityprovider.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/kmsconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/kubevirtplatformstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/ldapattributemapping.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/ldapidentityprovider.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/loadbalancer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/maxagepolicy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/mtumigration.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/mtumigrationvalues.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/network.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/networkdiagnostics.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/networkdiagnosticssourceplacement.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/networkdiagnosticstargetplacement.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/networkmigration.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/networkspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/networkstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/node.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/nodespec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/nodestatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/nutanixfailuredomain.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/nutanixplatformloadbalancer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/nutanixplatformspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/nutanixplatformstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/nutanixprismelementendpoint.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/nutanixprismendpoint.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/nutanixresourceidentifier.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/oauth.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/oauthremoteconnectioninfo.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/oauthspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/oauthtemplates.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/objectreference.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/oidcclientconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/oidcclientreference.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/oidcclientstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/oidcprovider.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/openidclaims.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/openididentityprovider.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/openstackplatformloadbalancer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/openstackplatformspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/openstackplatformstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/operandversion.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/operatorhub.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/operatorhubspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/operatorhubstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/ovirtplatformloadbalancer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/ovirtplatformstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/persistentvolumeclaimreference.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/persistentvolumeconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/pkicertificatesubject.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/platformspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/platformstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/policyfulciosubject.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/policyidentity.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/policymatchexactrepository.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/policymatchremapidentity.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/policyrootoftrust.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/powervsplatformspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/powervsplatformstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/powervsserviceendpoint.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/prefixedclaimmapping.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/profilecustomizations.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/project.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/projectspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/promqlclustercondition.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/proxy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/proxyspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/proxystatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/registrylocation.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/registrysources.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/release.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/repositorydigestmirrors.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/requestheaderidentityprovider.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/requiredhstspolicy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/scheduler.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/schedulerspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/secretnamereference.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/signaturestore.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/storage.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/templatereference.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/tlsprofilespec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/tlssecurityprofile.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/tokenclaimmapping.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/tokenclaimmappings.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/tokenclaimorexpressionmapping.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/tokenclaimvalidationcelrule.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/tokenclaimvalidationrule.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/tokenconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/tokenissuer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/tokenrequiredclaim.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/tokenuservalidationrule.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/update.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/updatehistory.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/usernameclaimmapping.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/usernameprefix.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/vaultapproleauthentication.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/vaultauthentication.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/vaultconfigmapreference.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/vaultkmsconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/vaultsecretreference.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/vaulttlsconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/vspherefailuredomainhostgroup.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/vspherefailuredomainregionaffinity.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/vspherefailuredomainzoneaffinity.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/vsphereplatformfailuredomainspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/vsphereplatformloadbalancer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/vsphereplatformnodenetworking.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/vsphereplatformnodenetworkingspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/vsphereplatformspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/vsphereplatformstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/vsphereplatformtopology.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/vsphereplatformvcenterspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/webhooktokenauthenticator.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/additionalalertmanagerconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/alertmanagerconfig.gois excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (32)
Makefilecmd/lifecycle-controller/main.gocmd/lifecycle-controller/start.gocmd/lifecycle-controller/util.gogo.modmanifests/0000_50_olm_08-lifecycle-controller.deployment.ibm-cloud-managed.yamlmanifests/0000_50_olm_08-lifecycle-controller.deployment.yamlmanifests/0000_50_olm_08-lifecycle-controller.networkpolicy.yamlmanifests/0000_50_olm_08-lifecycle-controller.rbac.yamlmanifests/0000_50_olm_08-lifecycle-controller.service.yamlmanifests/0000_50_olm_09-lifecycle-server.rbac.yamlmicroshift-manifests/0000_50_olm_08-lifecycle-controller.deployment.ibm-cloud-managed.yamlmicroshift-manifests/0000_50_olm_08-lifecycle-controller.deployment.yamlmicroshift-manifests/0000_50_olm_08-lifecycle-controller.networkpolicy.yamlmicroshift-manifests/0000_50_olm_08-lifecycle-controller.rbac.yamlmicroshift-manifests/0000_50_olm_08-lifecycle-controller.service.yamlmicroshift-manifests/0000_50_olm_09-lifecycle-server.rbac.yamlmicroshift-manifests/kustomization.yamloperator-lifecycle-manager.Dockerfilepkg/lifecycle-controller/TODO.mdpkg/lifecycle-controller/controller.gopkg/lifecycle-controller/controller_test.gopkg/lifecycle-controller/tls.gopkg/lifecycle-controller/tls_test.goscripts/generate_crds_manifests.shstaging/operator-lifecycle-manager/test/e2e/downstream_lifecycle_controller_test.gostaging/operator-lifecycle-manager/test/e2e/downstream_lifecycle_server_test.gostaging/operator-lifecycle-manager/test/images/downstream-lifecycle-controller/build.shstaging/operator-lifecycle-manager/test/images/downstream-lifecycle-controller/lifecycle-catalog-no-lifecycle/Dockerfilestaging/operator-lifecycle-manager/test/images/downstream-lifecycle-controller/lifecycle-catalog-no-lifecycle/configs/catalog.jsonstaging/operator-lifecycle-manager/test/images/downstream-lifecycle-controller/lifecycle-catalog/Dockerfilestaging/operator-lifecycle-manager/test/images/downstream-lifecycle-controller/lifecycle-catalog/configs/catalog.json
✅ Files skipped from review due to trivial changes (12)
- operator-lifecycle-manager.Dockerfile
- cmd/lifecycle-controller/main.go
- manifests/0000_50_olm_09-lifecycle-server.rbac.yaml
- microshift-manifests/0000_50_olm_09-lifecycle-server.rbac.yaml
- microshift-manifests/0000_50_olm_08-lifecycle-controller.networkpolicy.yaml
- cmd/lifecycle-controller/util.go
- pkg/lifecycle-controller/tls_test.go
- microshift-manifests/kustomization.yaml
- manifests/0000_50_olm_08-lifecycle-controller.deployment.ibm-cloud-managed.yaml
- pkg/lifecycle-controller/TODO.md
- staging/operator-lifecycle-manager/test/images/downstream-lifecycle-controller/build.sh
- go.mod
🚧 Files skipped from review as they are similar to previous changes (7)
- manifests/0000_50_olm_08-lifecycle-controller.networkpolicy.yaml
- manifests/0000_50_olm_08-lifecycle-controller.service.yaml
- Makefile
- microshift-manifests/0000_50_olm_08-lifecycle-controller.deployment.ibm-cloud-managed.yaml
- staging/operator-lifecycle-manager/test/e2e/downstream_lifecycle_controller_test.go
- manifests/0000_50_olm_08-lifecycle-controller.deployment.yaml
- microshift-manifests/0000_50_olm_08-lifecycle-controller.service.yaml
openshift-ci
Bot
added
the
needs-rebase
perdasilva
force-pushed
the
lifecycle-controller
branch
6 times, most recently
from
2f555a5 to
6ef40ba
Compare
openshift-ci
Bot
removed
the
needs-rebase
Contributor
Author
|
@coderabbitai resume |
There was a problem hiding this comment.
🧹 Nitpick comments (7)
microshift-manifests/0000_50_olm_08-lifecycle-controller.deployment.yaml (1)
62-63: 💤 Low valueVerify GOMEMLIMIT of 5MiB is sufficient for the lifecycle-controller.
The
GOMEMLIMIT=5MiBis quite low for a controller-runtime based application. While the memory request is 10Mi, controller-runtime's informer caches can grow depending on the number of watched objects (CatalogSources, Pods, Deployments, etc.). If the cluster has many catalog sources or resources, GC pressure could become significant.Consider monitoring memory usage in practice or increasing to a more comfortable threshold (e.g., 8MiB or matching the request).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@microshift-manifests/0000_50_olm_08-lifecycle-controller.deployment.yaml` around lines 62 - 63, The GOMEMLIMIT environment variable for the lifecycle-controller is set to an unusually low "5MiB"; update the container's env entry (GOMEMLIMIT) in the lifecycle-controller deployment manifest to a higher, safer value such as "8MiB" or one that matches the memory request (e.g., "10MiB") to reduce GC pressure for controller-runtime informer caches and monitor memory usage after deployment.pkg/lifecycle-controller/controller_test.go (1)
168-172: 💤 Low valueTest case "distinct long names produce different results" is self-referential.
The expected value calls
resourceName(...)with the same input as the test, which makes this test case a tautology—it will always pass regardless of implementation correctness. Consider using a pre-computed expected value instead.However, the actual collision prevention is validated in
TestResourceName_NoCollision(Lines 184-190), which correctly tests two distinct long names produce different outputs. This test case appears to be a placeholder for documenting the pattern rather than a strict assertion.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/lifecycle-controller/controller_test.go` around lines 168 - 172, The test case named "distinct long names produce different results" is tautological because it computes expected by calling resourceName on the same input; change it to assert against a concrete, precomputed expected string instead of calling resourceName(input). Locate the test case in controller_test.go and replace expected: resourceName("this-is-a-very-long-catalog-source-name-that-exceeds-the-dns-xxxxx") with the actual expected resource name value (the precomputed hashed/truncated result your resourceName implementation should produce) so the test fails if resourceName regresses; keep the test name and input unchanged and only modify the expected value to a constant string.pkg/lifecycle-controller/controller.go (1)
726-741: 💤 Low valueTLS profile change handler logs error but returns nil, silently dropping the requeue.
When listing CatalogSources fails in the TLS change handler (Line 730-733), the error is logged but the function returns
nil, meaning no reconciliation requests are enqueued. A transient API server error during TLS profile changes could leave lifecycle-server deployments running with stale TLS configurations until the next unrelated reconcile.Consider returning a sentinel request that triggers a delayed requeue, or relying on the periodic resync if that's acceptable for TLS propagation latency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/lifecycle-controller/controller.go` around lines 726 - 741, The TLS-profile change handler currently logs errors from mgr.GetClient().List but returns nil, dropping reconciliation; modify the TypedEnqueueRequestsFromMapFunc handler (the closure passed to source.Channel(tlsProfileChan, handler.TypedEnqueueRequestsFromMapFunc(...))) so that when mgr.GetClient().List(ctx, &catalogSources) returns an error you return a sentinel reconcile.Request to force a retry (e.g., a single request that will requeue the controller instead of an empty slice), and keep r.Log.Error(...) for visibility; ensure you still return the full list of reconcile.Request for the successful-list path so CatalogSource objects are enqueued as before.staging/operator-lifecycle-manager/test/images/downstream-lifecycle-controller/Dockerfile (2)
3-3: ⚡ Quick winPin the base image to an immutable digest instead of
:latest.Using
:latestmakes test behavior drift over time and can cause non-reproducible CI results.Proposed change
-FROM quay.io/operator-framework/opm:latest +FROM quay.io/operator-framework/opm@sha256:<resolved-digest>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@staging/operator-lifecycle-manager/test/images/downstream-lifecycle-controller/Dockerfile` at line 3, The Dockerfile currently uses a floating tag "FROM quay.io/operator-framework/opm:latest"; change this to a pinned immutable digest by replacing that FROM line with the same image referenced by its sha256 digest (e.g., quay.io/operator-framework/opm@sha256:...) so the base image is fixed and CI/tests are reproducible—update the FROM instruction in the Dockerfile accordingly and commit the pinned digest.
3-15: ⚡ Quick winSet an explicit non-root runtime user.
Please set
USERexplicitly so this image remains compliant with restricted policies even if base-image defaults change.Proposed change
FROM quay.io/operator-framework/opm:latest @@ RUN ["/bin/opm", "serve", "/configs", "--cache-dir=/tmp/cache", "--cache-only"] +USER 65532:65532🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@staging/operator-lifecycle-manager/test/images/downstream-lifecycle-controller/Dockerfile` around lines 3 - 15, The image lacks an explicit non-root runtime user: add a USER instruction with a non-root uid/gid (e.g., 1000 or 65534) after the file setup steps and before runtime execution, and update the Dockerfile so the created runtime user can access the copied files by chown-ing /configs and the cache dir in the RUN stage (ensure permissions for /tmp/cache and /configs); keep ENTRYPOINT ["/bin/opm"], CMD ["serve"...] and the LABEL unchanged so the container runs as the non-root user.staging/operator-lifecycle-manager/test/images/downstream-lifecycle-controller/lifecycle-catalog-no-lifecycle/configs/catalog.json (1)
1-3: ⚡ Quick winExclude NDJSON catalog fixtures from strict single-JSON linting.
This fixture is newline-delimited JSON records, so strict JSON parsers (like Biome in single-document mode) will keep flagging parse errors. Consider excluding this path from that rule (or treating these fixtures as JSONL) to avoid noisy failures.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@staging/operator-lifecycle-manager/test/images/downstream-lifecycle-controller/lifecycle-catalog-no-lifecycle/configs/catalog.json` around lines 1 - 3, This NDJSON fixture contains newline-delimited JSON records (schemas "olm.package", "olm.channel", "olm.bundle") and should be excluded from strict single-JSON linting; update the lint configuration (or Biome rule set) to either treat these catalog fixtures as JSONL or add a rule/ignore entry that excludes files matching this catalog pattern (the catalog.json NDJSON fixture containing entries with "olm.package"/"olm.channel"/"olm.bundle") so the linter no longer enforces single-document JSON parsing on these fixtures.pkg/lifecycle-server/fbc.go (1)
117-123: ⚡ Quick winReturn lifecycle versions in deterministic order.
ListVersions()currently depends on map iteration order, which is nondeterministic. Sorting here avoids unstable logs and caller behavior.♻️ Proposed fix
import ( "context" "encoding/json" "fmt" "os" "regexp" + "sort" "sync" @@ func (index LifecycleIndex) ListVersions() []string { versions := make([]string, 0, len(index)) for v := range index { versions = append(versions, v) } + sort.Strings(versions) return versions }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/lifecycle-server/fbc.go` around lines 117 - 123, ListVersions currently returns map keys in nondeterministic order; modify LifecycleIndex.ListVersions to produce a deterministic sorted slice by collecting keys into versions and calling sort.Strings(versions) before returning. Update the function that builds versions (ListVersions) and import the sort package if missing so callers receive a stable, sorted list of lifecycle versions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@microshift-manifests/0000_50_olm_08-lifecycle-controller.deployment.yaml`:
- Around line 62-63: The GOMEMLIMIT environment variable for the
lifecycle-controller is set to an unusually low "5MiB"; update the container's
env entry (GOMEMLIMIT) in the lifecycle-controller deployment manifest to a
higher, safer value such as "8MiB" or one that matches the memory request (e.g.,
"10MiB") to reduce GC pressure for controller-runtime informer caches and
monitor memory usage after deployment.
In `@pkg/lifecycle-controller/controller_test.go`:
- Around line 168-172: The test case named "distinct long names produce
different results" is tautological because it computes expected by calling
resourceName on the same input; change it to assert against a concrete,
precomputed expected string instead of calling resourceName(input). Locate the
test case in controller_test.go and replace expected:
resourceName("this-is-a-very-long-catalog-source-name-that-exceeds-the-dns-xxxxx")
with the actual expected resource name value (the precomputed hashed/truncated
result your resourceName implementation should produce) so the test fails if
resourceName regresses; keep the test name and input unchanged and only modify
the expected value to a constant string.
In `@pkg/lifecycle-controller/controller.go`:
- Around line 726-741: The TLS-profile change handler currently logs errors from
mgr.GetClient().List but returns nil, dropping reconciliation; modify the
TypedEnqueueRequestsFromMapFunc handler (the closure passed to
source.Channel(tlsProfileChan, handler.TypedEnqueueRequestsFromMapFunc(...))) so
that when mgr.GetClient().List(ctx, &catalogSources) returns an error you return
a sentinel reconcile.Request to force a retry (e.g., a single request that will
requeue the controller instead of an empty slice), and keep r.Log.Error(...) for
visibility; ensure you still return the full list of reconcile.Request for the
successful-list path so CatalogSource objects are enqueued as before.
In `@pkg/lifecycle-server/fbc.go`:
- Around line 117-123: ListVersions currently returns map keys in
nondeterministic order; modify LifecycleIndex.ListVersions to produce a
deterministic sorted slice by collecting keys into versions and calling
sort.Strings(versions) before returning. Update the function that builds
versions (ListVersions) and import the sort package if missing so callers
receive a stable, sorted list of lifecycle versions.
In
`@staging/operator-lifecycle-manager/test/images/downstream-lifecycle-controller/Dockerfile`:
- Line 3: The Dockerfile currently uses a floating tag "FROM
quay.io/operator-framework/opm:latest"; change this to a pinned immutable digest
by replacing that FROM line with the same image referenced by its sha256 digest
(e.g., quay.io/operator-framework/opm@sha256:...) so the base image is fixed and
CI/tests are reproducible—update the FROM instruction in the Dockerfile
accordingly and commit the pinned digest.
- Around line 3-15: The image lacks an explicit non-root runtime user: add a
USER instruction with a non-root uid/gid (e.g., 1000 or 65534) after the file
setup steps and before runtime execution, and update the Dockerfile so the
created runtime user can access the copied files by chown-ing /configs and the
cache dir in the RUN stage (ensure permissions for /tmp/cache and /configs);
keep ENTRYPOINT ["/bin/opm"], CMD ["serve"...] and the LABEL unchanged so the
container runs as the non-root user.
In
`@staging/operator-lifecycle-manager/test/images/downstream-lifecycle-controller/lifecycle-catalog-no-lifecycle/configs/catalog.json`:
- Around line 1-3: This NDJSON fixture contains newline-delimited JSON records
(schemas "olm.package", "olm.channel", "olm.bundle") and should be excluded from
strict single-JSON linting; update the lint configuration (or Biome rule set) to
either treat these catalog fixtures as JSONL or add a rule/ignore entry that
excludes files matching this catalog pattern (the catalog.json NDJSON fixture
containing entries with "olm.package"/"olm.channel"/"olm.bundle") so the linter
no longer enforces single-document JSON parsing on these fixtures.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 77cec337-9795-4712-9959-918696786f17
⛔ Files ignored due to path filters (224)
go.sumis excluded by!**/*.sumvendor/github.com/openshift/api/config/v1/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_apiserver.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_authentication.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_cluster_version.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_dns.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_infrastructure.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_ingress.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_insights.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_kmsencryption.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_network.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_tlssecurityprofile.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1alpha1/register.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/types_cluster_image_policy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/types_cluster_monitoring.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/types_crio_credential_provider_config.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/types_image_policy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/types_insights.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/types_pki.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1alpha1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1alpha1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1alpha2/types_insights.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha2/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/acceptrisk.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/alibabacloudplatformstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/alibabacloudresourcetag.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/apiserver.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/apiserverencryption.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/apiservernamedservingcert.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/apiserverservingcerts.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/apiserverspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/audit.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/auditcustomrule.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/authentication.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/authenticationspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/authenticationstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/awsdnsspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/awsingressspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/awskmsconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/awsplatformspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/awsplatformstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/awsresourcetag.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/awsserviceendpoint.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/azureplatformstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/azureresourcetag.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/baremetalplatformloadbalancer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/baremetalplatformspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/baremetalplatformstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/basicauthidentityprovider.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/build.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/builddefaults.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/buildoverrides.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/buildspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/cloudcontrollermanagerstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/cloudloadbalancerconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/cloudloadbalancerips.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/clustercondition.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/clusterimagepolicy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/clusterimagepolicyspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/clusterimagepolicystatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/clusternetworkentry.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/clusteroperator.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/clusteroperatorstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/clusteroperatorstatuscondition.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/clusterversion.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/clusterversioncapabilitiesspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/clusterversioncapabilitiesstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/clusterversionspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/clusterversionstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/componentoverride.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/componentroutespec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/componentroutestatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/conditionalupdate.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/conditionalupdaterisk.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/configmapfilereference.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/configmapnamereference.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/console.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/consoleauthentication.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/consolespec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/consolestatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/custom.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/customfeaturegates.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/customtlsprofile.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/deprecatedwebhooktokenauthenticator.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/dns.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/dnsplatformspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/dnsspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/dnszone.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/equinixmetalplatformstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/externalipconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/externalippolicy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/externalplatformspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/externalplatformstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/extramapping.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/featuregate.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/featuregateattributes.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/featuregatedetails.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/featuregateselection.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/featuregatestatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/gatherconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/gathererconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/gatherers.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/gcpplatformstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/gcpresourcelabel.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/gcpresourcetag.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/githubidentityprovider.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/gitlabidentityprovider.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/googleidentityprovider.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/htpasswdidentityprovider.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/hubsource.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/hubsourcestatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/ibmcloudplatformspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/ibmcloudplatformstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/ibmcloudserviceendpoint.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/identityprovider.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/identityproviderconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/image.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagecontentpolicy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagecontentpolicyspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagedigestmirrors.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagedigestmirrorset.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagedigestmirrorsetspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagelabel.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagepolicy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagepolicyfulciocawithrekorrootoftrust.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagepolicypkirootoftrust.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagepolicypublickeyrootoftrust.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagepolicyspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagepolicystatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagesigstoreverificationpolicy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagespec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagestatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagetagmirrors.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagetagmirrorset.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagetagmirrorsetspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/infrastructure.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/infrastructurespec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/infrastructurestatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/ingress.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/ingressplatformspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/ingressspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/ingressstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/insightsdatagather.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/insightsdatagatherspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/keystoneidentityprovider.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/kmsconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/kubevirtplatformstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/ldapattributemapping.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/ldapidentityprovider.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/loadbalancer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/maxagepolicy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/mtumigration.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/mtumigrationvalues.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/network.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/networkdiagnostics.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/networkdiagnosticssourceplacement.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/networkdiagnosticstargetplacement.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/networkmigration.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/networkspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/networkstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/node.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/nodespec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/nodestatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/nutanixfailuredomain.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/nutanixplatformloadbalancer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/nutanixplatformspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/nutanixplatformstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/nutanixprismelementendpoint.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/nutanixprismendpoint.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/nutanixresourceidentifier.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/oauth.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/oauthremoteconnectioninfo.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/oauthspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/oauthtemplates.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/objectreference.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/oidcclientconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/oidcclientreference.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/oidcclientstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/oidcprovider.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/openidclaims.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/openididentityprovider.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/openstackplatformloadbalancer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/openstackplatformspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/openstackplatformstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/operandversion.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/operatorhub.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/operatorhubspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/operatorhubstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/ovirtplatformloadbalancer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/ovirtplatformstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/persistentvolumeclaimreference.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/persistentvolumeconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/pkicertificatesubject.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/platformspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/platformstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/policyfulciosubject.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/policyidentity.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/policymatchexactrepository.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/policymatchremapidentity.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/policyrootoftrust.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/powervsplatformspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/powervsplatformstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/powervsserviceendpoint.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/prefixedclaimmapping.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/profilecustomizations.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/project.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/projectspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/promqlclustercondition.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/proxy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/proxyspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/proxystatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/registrylocation.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/registrysources.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/release.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/repositorydigestmirrors.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/requestheaderidentityprovider.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/requiredhstspolicy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/scheduler.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/schedulerspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/secretnamereference.gois excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (37)
Makefilecmd/lifecycle-controller/main.gocmd/lifecycle-controller/start.gocmd/lifecycle-controller/util.gocmd/lifecycle-server/main.gocmd/lifecycle-server/start.gogo.modmanifests/0000_50_olm_08-lifecycle-controller.deployment.ibm-cloud-managed.yamlmanifests/0000_50_olm_08-lifecycle-controller.deployment.yamlmanifests/0000_50_olm_08-lifecycle-controller.networkpolicy.yamlmanifests/0000_50_olm_08-lifecycle-controller.rbac.yamlmanifests/0000_50_olm_08-lifecycle-controller.service.yamlmanifests/0000_50_olm_09-lifecycle-server.rbac.yamlmicroshift-manifests/0000_50_olm_08-lifecycle-controller.deployment.ibm-cloud-managed.yamlmicroshift-manifests/0000_50_olm_08-lifecycle-controller.deployment.yamlmicroshift-manifests/0000_50_olm_08-lifecycle-controller.networkpolicy.yamlmicroshift-manifests/0000_50_olm_08-lifecycle-controller.rbac.yamlmicroshift-manifests/0000_50_olm_08-lifecycle-controller.service.yamlmicroshift-manifests/0000_50_olm_09-lifecycle-server.rbac.yamlmicroshift-manifests/kustomization.yamloperator-lifecycle-manager.Dockerfilepkg/lifecycle-controller/TODO.mdpkg/lifecycle-controller/controller.gopkg/lifecycle-controller/controller_test.gopkg/lifecycle-controller/tls.gopkg/lifecycle-controller/tls_test.gopkg/lifecycle-server/fbc.gopkg/lifecycle-server/fbc_test.gopkg/lifecycle-server/server.gopkg/lifecycle-server/server_test.goscripts/generate_crds_manifests.shstaging/operator-lifecycle-manager/test/e2e/downstream_lifecycle_controller_test.gostaging/operator-lifecycle-manager/test/e2e/downstream_lifecycle_server_test.gostaging/operator-lifecycle-manager/test/images/downstream-lifecycle-controller/Dockerfilestaging/operator-lifecycle-manager/test/images/downstream-lifecycle-controller/build.shstaging/operator-lifecycle-manager/test/images/downstream-lifecycle-controller/lifecycle-catalog-no-lifecycle/configs/catalog.jsonstaging/operator-lifecycle-manager/test/images/downstream-lifecycle-controller/lifecycle-catalog/configs/catalog.json
✅ Files skipped from review due to trivial changes (10)
- cmd/lifecycle-server/main.go
- operator-lifecycle-manager.Dockerfile
- microshift-manifests/0000_50_olm_09-lifecycle-server.rbac.yaml
- microshift-manifests/0000_50_olm_08-lifecycle-controller.service.yaml
- manifests/0000_50_olm_09-lifecycle-server.rbac.yaml
- microshift-manifests/0000_50_olm_08-lifecycle-controller.rbac.yaml
- staging/operator-lifecycle-manager/test/e2e/downstream_lifecycle_controller_test.go
- microshift-manifests/kustomization.yaml
- staging/operator-lifecycle-manager/test/e2e/downstream_lifecycle_server_test.go
- pkg/lifecycle-controller/TODO.md
🚧 Files skipped from review as they are similar to previous changes (10)
- manifests/0000_50_olm_08-lifecycle-controller.service.yaml
- manifests/0000_50_olm_08-lifecycle-controller.networkpolicy.yaml
- pkg/lifecycle-server/fbc_test.go
- manifests/0000_50_olm_08-lifecycle-controller.deployment.ibm-cloud-managed.yaml
- cmd/lifecycle-server/start.go
- pkg/lifecycle-server/server.go
- staging/operator-lifecycle-manager/test/images/downstream-lifecycle-controller/build.sh
- Makefile
- go.mod
- scripts/generate_crds_manifests.sh
perdasilva
force-pushed
the
lifecycle-controller
branch
11 times, most recently
from
f05c3fc to
4eac709
Compare
Contributor
Author
|
@coderabbitai resume |
|
No description provided. |
✅ Actions performedReviews resumed. |
Contributor
Author
|
/unhold |
openshift-ci
Bot
removed
the
do-not-merge/hold
perdasilva
force-pushed
the
lifecycle-controller
branch
from
4eac709 to
5ec530e
Compare
Contributor
Author
|
/retest |
perdasilva
force-pushed
the
lifecycle-controller
branch
from
5ec530e to
3491ce7
Compare
openshift-ci
Bot
added
the
needs-rebase
Bumps vendored dependencies to compatible versions: - openshift/api v0.0.0-20260429211050 - openshift/library-go v0.0.0-20260213153706 Removes the controller-runtime-common replace directive by using the published openshift/controller-runtime-common module directly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
perdasilva
force-pushed
the
lifecycle-controller
branch
from
3491ce7 to
ec7e58a
Compare
openshift-ci
Bot
removed
the
needs-rebase
Contributor
Author
|
/retest |
perdasilva
force-pushed
the
lifecycle-controller
branch
from
ec7e58a to
6a41470
Compare
Introduces a lifecycle-controller that watches CatalogSources and manages per-catalog lifecycle-server deployments. For each matching CatalogSource with a running pod, the controller creates a Deployment, ServiceAccount, Service, and NetworkPolicy using server-side apply, and maintains a shared ClusterRoleBinding across all lifecycle-server instances. Key components: - cmd/lifecycle-controller: CLI entrypoint with TLS profile watching, leader election, metrics serving, and health/readiness probes - pkg/lifecycle-controller: Reconciler with SSA-based resource management, thread-safe TLS config provider, and catalog pod image extraction - RBAC, Deployment, Service, NetworkPolicy manifests for build/deployment (gated behind TechPreviewNoUpgrade) Also vendors controller-runtime-common for OpenShift TLS profile support. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
perdasilva
force-pushed
the
lifecycle-controller
branch
from
6a41470 to
756628f
Compare
Contributor
|
@perdasilva: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. |
Member
|
/test e2e-gcp-olm |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Introduces a lifecycle-controller that watches CatalogSources and manages per-catalog lifecycle-server deployments with their supporting infrastructure. Manifests and deployment configuration are tracked in a separate PR.
For each matching CatalogSource with a running catalog pod, the controller creates a Deployment, ServiceAccount, Service, and NetworkPolicy using server-side apply. A shared ClusterRoleBinding aggregates all lifecycle-server ServiceAccounts and is reconciled on every change. Namespace-scoped resources are cleaned up automatically via Kubernetes garbage collection (owner references); the ClusterRoleBinding — which cannot use owner references across scope boundaries — is maintained through a finalizer on matching CatalogSources.
Resource lifecycle
MergeFrompatch instead of full object Update to avoid conflict errors from concurrent metadata changesreconcileClusterRoleBindingskips CatalogSources with a DeletionTimestamp set, ensuring the subject is removed during the finalizer flow rather than relying solely on GC timingOperational details
--catalog-source-field-selectorflag (typicallymetadata.namespace=openshift-marketplace), reducing memory footprint on clusters with many CatalogSourcesCommits
vendor: update openshift/api and openshift/library-go— vendored dependencies for TLS profile supportfeat: add lifecycle-controller for managing catalog lifecycle pods— controller code, unit tests, Makefile, DockerfileKey Components
cmd/lifecycle-controller/— CLI entrypoint with TLS profile watching, leader election, metrics serving with authn/authz, health/readiness probespkg/lifecycle-controller/controller.go— SSA-based reconciler: creates/updates lifecycle-server resources per CatalogSource, manages shared ClusterRoleBinding, relies on owner references for GC-based cleanuppkg/lifecycle-controller/tls.go— Thread-safe TLS config provider that dynamically updates when the cluster TLS profile changesCreated Resources (per CatalogSource)
serving-cert-secret-nametriggers cert generationTest plan
go build ./cmd/lifecycle-controller/...succeedsgo test ./pkg/lifecycle-controller/...passes (33/33)🤖 Generated with Claude Code