NO-ISSUE: Synchronize From Upstream Repositories#1338
Conversation
|
@openshift-bot: This pull request explicitly references no 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. |
|
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:
WalkthroughDependency versions and YAML module usage were updated, CI actions were upgraded, manifest loaders now propagate walk errors, registry credential handling and substitution cycle detection were added, and catalog/bundle network policies were introduced. ChangesCore behavior and validation
Dependency, CI, and test maintenance
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related issues
Suggested reviewers: 🚥 Pre-merge checks | ✅ 13 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (13 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
/retest |
636c4d3 to
0711859
Compare
|
/retest |
0711859 to
ece9ee5
Compare
|
/retest |
ece9ee5 to
eb08d90
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@staging/operator-lifecycle-manager/deploy/chart/templates/0000_50_olm_01-networkpolicies.yaml`:
- Around line 113-132: Replace the wildcard egress rule in the
bundle-unpack-egress NetworkPolicy with explicit rules reusing
.Values.networkPolicy.kubeAPIServer and .Values.networkPolicy.dns, plus only the
required registry and object-store destinations. Preserve the existing pod
selectors and policy type, and ensure no unrestricted destination or port
remains.
- Around line 89-111: The olm-catalog-grpc-ingress NetworkPolicy is rendered for
unsupported split-namespace deployments. Gate the manifest, including its
metadata and spec, on .Values.catalog_namespace equaling .Values.namespace so it
is omitted when the namespaces differ; preserve the existing policy unchanged
for matching namespaces.
🪄 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: 46a3c27d-2b80-473c-aa67-127bef63cebb
⛔ Files ignored due to path filters (129)
go.sumis excluded by!**/*.sumstaging/operator-lifecycle-manager/go.sumis excluded by!**/*.sumvendor/github.com/go-logr/logr/context_noslog.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-logr/logr/context_slog.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-logr/logr/funcr/funcr.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-logr/logr/funcr/slogsink.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-logr/logr/sloghandler.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-logr/logr/slogr.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-logr/logr/slogr/slogr.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-logr/logr/slogsink.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/flate/dict_decoder.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/flate/inflate.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/huff0/build_table.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/internal/snapref/decode.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/dict.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/enc_base.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/enc_best.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/enc_better.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/enc_dfast.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/enc_fast.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/enc_jobs.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/encoder.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/encoder_options.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/fse_decoder_amd64.sis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/fse_decoder_arm64.sis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/fse_decoder_asm.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/fse_decoder_generic.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/seqdec_amd64.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/seqdec_amd64.sis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/seqdec_arm64.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/seqdec_arm64.sis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/seqdec_asm.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/seqdec_generic.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/operator-framework/operator-lifecycle-manager/util/cpb/main.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/internal/github.com/golang/gddo/httputil/header/header.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/collectors/go_collector_go116.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/collectors/go_collector_latest.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/counter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/desc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/expvar_collector.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/gauge.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/go_collector_go116.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/go_collector_latest.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/histogram.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/internal/difflib.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/labels.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/metric.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/process_collector_darwin.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/process_collector_windows.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/promhttp/http.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/promhttp/instrument_client.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/promhttp/instrument_server.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/promhttp/option.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/registry.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/summary.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/timer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/vec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/wrap.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/Makefile.commonis excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/SECURITY.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/crypto.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/mountinfo.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/net_wireless.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/proc_cgroup.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/crypto/openpgp/armor/armor.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/crypto/openpgp/errors/errors.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/crypto/openpgp/packet/packet.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/crypto/openpgp/read.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/crypto/openpgp/s2k/s2k.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/http2/transport_wrap.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/idna/idna.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sync/semaphore/semaphore.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/cpu/parse.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/syscall_linux.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/syscall_linux_386.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/syscall_linux_amd64.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/syscall_linux_arm.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/syscall_linux_arm64.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/syscall_linux_loong64.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/syscall_linux_mips64x.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/syscall_linux_mipsx.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/syscall_linux_ppc.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/syscall_linux_ppc64x.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/syscall_linux_riscv64.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/syscall_linux_s390x.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/syscall_linux_sparc64.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zerrors_linux.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zsyscall_linux.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zsyscall_linux_386.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zsyscall_linux_amd64.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zsyscall_linux_arm.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zsyscall_linux_arm64.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zsyscall_linux_loong64.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zsyscall_linux_mips.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zsyscall_linux_mips64.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zsyscall_linux_mips64le.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zsyscall_linux_mipsle.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zsyscall_linux_ppc.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zsyscall_linux_ppc64.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zsyscall_linux_ppc64le.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zsyscall_linux_riscv64.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zsyscall_linux_s390x.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zsyscall_linux_sparc64.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/windows/security_windows.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/windows/syscall_windows.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/windows/types_windows.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/cases/context.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/cases/map.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/unicode/norm/forminfo.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/unicode/norm/iter.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/unicode/norm/normalize.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/packages/packages.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/gcimporter/iexport.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/gcimporter/iimport.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/imports/fix.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/imports/imports.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/stdlib/deps.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/stdlib/manifest.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/typesinternal/element.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/typesinternal/types.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/typesinternal/zerovalue.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/envconfig/envconfig.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/transport/controlbuf.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/transport/http2_client.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/transport/http2_server.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/version.gois excluded by!**/vendor/**,!vendor/**vendor/modules.txtis excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (11)
go.modmanifests/0000_50_olm_01-networkpolicies.yamlmicroshift-manifests/0000_50_olm_01-networkpolicies.yamlstaging/operator-lifecycle-manager/.github/workflows/e2e-tests.ymlstaging/operator-lifecycle-manager/.github/workflows/goreleaser.yamlstaging/operator-lifecycle-manager/.github/workflows/sanity.yamlstaging/operator-lifecycle-manager/.github/workflows/unit.ymlstaging/operator-lifecycle-manager/deploy/chart/templates/0000_50_olm_01-networkpolicies.yamlstaging/operator-lifecycle-manager/go.modstaging/operator-lifecycle-manager/pkg/controller/operators/catalog/operator_test.gostaging/operator-lifecycle-manager/util/cpb/main.go
🚧 Files skipped from review as they are similar to previous changes (10)
- staging/operator-lifecycle-manager/.github/workflows/sanity.yaml
- staging/operator-lifecycle-manager/.github/workflows/unit.yml
- staging/operator-lifecycle-manager/.github/workflows/goreleaser.yaml
- staging/operator-lifecycle-manager/util/cpb/main.go
- staging/operator-lifecycle-manager/.github/workflows/e2e-tests.yml
- go.mod
- microshift-manifests/0000_50_olm_01-networkpolicies.yaml
- manifests/0000_50_olm_01-networkpolicies.yaml
- staging/operator-lifecycle-manager/pkg/controller/operators/catalog/operator_test.go
- staging/operator-lifecycle-manager/go.mod
| # Complements per-CatalogSource NPs from the controller; covers the bootstrapping window before reconciliation. | ||
| # Effective only when catalog_namespace == namespace (the default). When they differ, no OLM-managed | ||
| # deny-all exists in catalog_namespace, so this rule has no effect; adding one there is unsafe since | ||
| # OLM does not exclusively own that namespace. | ||
| # No 'from:' restriction is intentional, matching current dynamic NP behavior. If the controller ever | ||
| # restricts ingress sources, this Helm-owned NP (which the controller cannot delete) will silently | ||
| # override that — treat as a permanent design constraint. | ||
| apiVersion: networking.k8s.io/v1 | ||
| kind: NetworkPolicy | ||
| metadata: | ||
| name: olm-catalog-grpc-ingress | ||
| namespace: {{ .Values.catalog_namespace }} | ||
| spec: | ||
| podSelector: | ||
| matchExpressions: | ||
| - key: olm.catalogSource | ||
| operator: Exists | ||
| policyTypes: | ||
| - Ingress | ||
| ingress: | ||
| - ports: | ||
| - protocol: TCP | ||
| port: {{ .Values.catalogGrpcPodPort }} |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Render this ingress policy only for the topology it supports.
Lines 90-92 claim this has no effect when catalog_namespace != namespace, but the manifest is rendered unconditionally. An ingress NetworkPolicy still selects matching pods and, absent another allow rule, denies ingress other than the listed TCP port. Split-namespace installations can therefore lose health, metrics, or service traffic. Gate this manifest on eq .Values.catalog_namespace .Values.namespace, or implement the correct split-namespace policy instead of documenting it as inert.
Proposed fix
+{{- if eq .Values.catalog_namespace .Values.namespace }}
---
apiVersion: networking.k8s.io/v1
kind: NetworkPolicy
...
ingress:
- ports:
- protocol: TCP
port: {{ .Values.catalogGrpcPodPort }}
+{{- end }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Complements per-CatalogSource NPs from the controller; covers the bootstrapping window before reconciliation. | |
| # Effective only when catalog_namespace == namespace (the default). When they differ, no OLM-managed | |
| # deny-all exists in catalog_namespace, so this rule has no effect; adding one there is unsafe since | |
| # OLM does not exclusively own that namespace. | |
| # No 'from:' restriction is intentional, matching current dynamic NP behavior. If the controller ever | |
| # restricts ingress sources, this Helm-owned NP (which the controller cannot delete) will silently | |
| # override that — treat as a permanent design constraint. | |
| apiVersion: networking.k8s.io/v1 | |
| kind: NetworkPolicy | |
| metadata: | |
| name: olm-catalog-grpc-ingress | |
| namespace: {{ .Values.catalog_namespace }} | |
| spec: | |
| podSelector: | |
| matchExpressions: | |
| - key: olm.catalogSource | |
| operator: Exists | |
| policyTypes: | |
| - Ingress | |
| ingress: | |
| - ports: | |
| - protocol: TCP | |
| port: {{ .Values.catalogGrpcPodPort }} | |
| {{- if eq .Values.catalog_namespace .Values.namespace }} | |
| # Complements per-CatalogSource NPs from the controller; covers the bootstrapping window before reconciliation. | |
| # Effective only when catalog_namespace == namespace (the default). When they differ, no OLM-managed | |
| # deny-all exists in catalog_namespace, so this rule has no effect; adding one there is unsafe since | |
| # OLM does not exclusively own that namespace. | |
| # No 'from:' restriction is intentional, matching current dynamic NP behavior. If the controller ever | |
| # restricts ingress sources, this Helm-owned NP (which the controller cannot delete) will silently | |
| # override that — treat as a permanent design constraint. | |
| apiVersion: networking.k8s.io/v1 | |
| kind: NetworkPolicy | |
| metadata: | |
| name: olm-catalog-grpc-ingress | |
| namespace: {{ .Values.catalog_namespace }} | |
| spec: | |
| podSelector: | |
| matchExpressions: | |
| - key: olm.catalogSource | |
| operator: Exists | |
| policyTypes: | |
| - Ingress | |
| ingress: | |
| - ports: | |
| - protocol: TCP | |
| port: {{ .Values.catalogGrpcPodPort }} | |
| {{- end }} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@staging/operator-lifecycle-manager/deploy/chart/templates/0000_50_olm_01-networkpolicies.yaml`
around lines 89 - 111, The olm-catalog-grpc-ingress NetworkPolicy is rendered
for unsupported split-namespace deployments. Gate the manifest, including its
metadata and spec, on .Values.catalog_namespace equaling .Values.namespace so it
is omitted when the namespaces differ; preserve the existing policy unchanged
for matching namespaces.
| # Wildcard egress; API server port omitted intentionally — it is implementation-defined and not statically knowable. | ||
| # Carries the same split-namespace limitation as olm-catalog-grpc-ingress above. | ||
| apiVersion: networking.k8s.io/v1 | ||
| kind: NetworkPolicy | ||
| metadata: | ||
| name: bundle-unpack-egress | ||
| namespace: {{ .Values.catalog_namespace }} | ||
| spec: | ||
| podSelector: | ||
| matchExpressions: | ||
| - key: operatorframework.io/bundle-unpack-ref | ||
| operator: Exists | ||
| - key: olm.managed | ||
| operator: In | ||
| values: | ||
| - "true" | ||
| policyTypes: | ||
| - Egress | ||
| egress: | ||
| - { } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Replace wildcard egress with an allowlist.
egress: - { } permits every destination and port for matching bundle-unpack pods. If the namespace is otherwise egress-isolated, this defeats the intended boundary and permits arbitrary cluster or external connections. Reuse the existing .Values.networkPolicy.kubeAPIServer and .Values.networkPolicy.dns configuration, then add only the required registry/object-store destinations.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@staging/operator-lifecycle-manager/deploy/chart/templates/0000_50_olm_01-networkpolicies.yaml`
around lines 113 - 132, Replace the wildcard egress rule in the
bundle-unpack-egress NetworkPolicy with explicit rules reusing
.Values.networkPolicy.kubeAPIServer and .Values.networkPolicy.dns, plus only the
required registry and object-store destinations. Preserve the existing pod
selectors and policy type, and ensure no unrestricted destination or port
remains.
|
/retest |
|
/lgtm |
Bumps [golang.org/x/sync](https://github.com/golang/sync) from 0.21.0 to 0.22.0. - [Commits](golang/sync@v0.21.0...v0.22.0) --- updated-dependencies: - dependency-name: golang.org/x/sync dependency-version: 0.22.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Upstream-repository: operator-lifecycle-manager Upstream-commit: 396954f2ceb5cc5e68ee364fb161525c05390b9e
Bumps [github.com/prometheus/common](https://github.com/prometheus/common) from 0.69.0 to 0.70.0. - [Release notes](https://github.com/prometheus/common/releases) - [Changelog](https://github.com/prometheus/common/blob/main/CHANGELOG.md) - [Commits](prometheus/common@v0.69.0...v0.70.0) --- updated-dependencies: - dependency-name: github.com/prometheus/common dependency-version: 0.70.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Upstream-repository: operator-lifecycle-manager Upstream-commit: fdd559459e09fce51148f8662f452018b55ea513
Bumps [golang.org/x/net](https://github.com/golang/net) from 0.56.0 to 0.57.0. - [Commits](golang/net@v0.56.0...v0.57.0) --- updated-dependencies: - dependency-name: golang.org/x/net dependency-version: 0.57.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Upstream-repository: operator-lifecycle-manager Upstream-commit: a4f060a9b8a125234a14099bb0f834219b5598eb
…ss and bundle unpack egress (#3863) * deploy/chart: add static NetworkPolicies for CatalogSource gRPC ingress and bundle unpack egress The Helm chart's default-deny-all-traffic policy blocked two critical traffic paths that are not covered by the existing static NetworkPolicies: 1. CatalogSource registry pods need to accept inbound gRPC connections on port 50051 from within the cluster. The catalog-operator reconciler already creates per-CatalogSource NetworkPolicies for this, but there is a bootstrapping gap between when default-deny-all-traffic is applied and when the controller first reconciles each CatalogSource. 2. Bundle-unpack Job pods need egress to reach the Kubernetes API server and container registries. The API server port is not statically specifiable because it varies across Kubernetes implementations, so a wildcard egress rule is used. Adds two new NetworkPolicies to the chart: - catalog-source-grpc-server: selects all pods carrying the olm.catalogSource label and allows ingress on the gRPC port. - bundle-unpack-egress: selects all pods carrying both the olm.managed=true and operatorframework.io/bundle-unpack-ref labels and allows unrestricted egress. Fixes: operator-framework/operator-lifecycle-manager#3676 Signed-off-by: grokspawn <jordan@nimblewidget.com> * deploy/chart: use catalog_namespace for CatalogSource and bundle-unpack NPs CatalogSource registry pods and bundle-unpack Jobs run in the namespace determined by .Values.catalog_namespace, not .Values.namespace. When a user overrides catalog_namespace to differ from namespace, the NetworkPolicies must be created in catalog_namespace to actually apply to those pods. Fixes review feedback on #3863. Signed-off-by: grokspawn <jordan@nimblewidget.com> --------- Signed-off-by: grokspawn <jordan@nimblewidget.com> Upstream-repository: operator-lifecycle-manager Upstream-commit: 174dccd0bd85f25fa9671839bbb21dc3a23cffc6
* chore: migrate deprecated gopkg.in/yaml.v3 to go.yaml.in/yaml/v3 Signed-off-by: Chiman Jain <chimanjain15@gmail.com> * chore: run go mod tidy Signed-off-by: Chiman Jain <chimanjain15@gmail.com> --------- Signed-off-by: Chiman Jain <chimanjain15@gmail.com> Upstream-repository: operator-lifecycle-manager Upstream-commit: b0123beceac05a5d08b6d5734b1b7772a4fda921
Bumps [github.com/prometheus/client_golang](https://github.com/prometheus/client_golang) from 1.23.2 to 1.24.0. - [Release notes](https://github.com/prometheus/client_golang/releases) - [Changelog](https://github.com/prometheus/client_golang/blob/v1.24.0/CHANGELOG.md) - [Commits](prometheus/client_golang@v1.23.2...v1.24.0) --- updated-dependencies: - dependency-name: github.com/prometheus/client_golang dependency-version: 1.24.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Upstream-repository: operator-lifecycle-manager Upstream-commit: f59f6ece21efeef554981a5dedb44f3304e51e15
Bumps [actions/setup-go](https://github.com/actions/setup-go) from 6 to 7. - [Release notes](https://github.com/actions/setup-go/releases) - [Commits](actions/setup-go@v6...v7) --- updated-dependencies: - dependency-name: actions/setup-go dependency-version: '7' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Upstream-repository: operator-lifecycle-manager Upstream-commit: 0a601aa745fa4293402df4a761f74de17c474400
Bumps [github.com/google/cel-go](https://github.com/google/cel-go) from 0.28.1 to 0.29.1. - [Release notes](https://github.com/google/cel-go/releases) - [Commits](cel-expr/cel-go@v0.28.1...v0.29.1) --- updated-dependencies: - dependency-name: github.com/google/cel-go dependency-version: 0.29.1 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Upstream-repository: api Upstream-commit: 132f4499f09433473b404d8da6594dd65aa89aba
Bumps [github.com/google/cel-go](https://github.com/google/cel-go) from 0.29.1 to 0.29.2. - [Release notes](https://github.com/google/cel-go/releases) - [Commits](cel-expr/cel-go@v0.29.1...v0.29.2) --- updated-dependencies: - dependency-name: github.com/google/cel-go dependency-version: 0.29.2 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Upstream-repository: api Upstream-commit: c4902decc528a59fb4efbcb33aef0c030978b845
Bumps [actions/setup-go](https://github.com/actions/setup-go) from 6 to 7. - [Release notes](https://github.com/actions/setup-go/releases) - [Commits](actions/setup-go@v6...v7) --- updated-dependencies: - dependency-name: actions/setup-go dependency-version: '7' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Upstream-repository: api Upstream-commit: 72d46e1db6dffddc1c5747c885d845b39d3db1b2
eb08d90 to
c6c6b0c
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED Approval requirements bypassed by manually added approval. This pull-request has been approved by: openshift-bot 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@staging/api/pkg/manifests/walkfunc_test.go`:
- Line 17: The assertions in walkfunc tests lack diagnostic context. Update the
affected require.ErrorIs, require.Len, require.Error, require.Contains, and
require.Mkdir calls to include concise, meaningful failure messages describing
the expected condition, while preserving their existing assertions and behavior.
- Line 81: Update both t.Cleanup callbacks around the permission restoration to
capture the os.Chmod error and report it through the test handle, such as
t.Errorf or t.Error. Ensure failures restoring each temporary directory’s
permissions are surfaced rather than discarded, while preserving the existing
cleanup behavior.
- Around line 58-72: Strengthen TestLoadBundle_NonexistentDirectory and
TestLoadPackage_NonexistentDirectory by asserting that each returned aggregate
error contains the expected filesystem walk failure from filepath.Walk, rather
than only checking that an error exists. Use the project’s existing
aggregate-error inspection and path-not-found cause symbols where available,
preserving the current nonexistent-directory setup.
🪄 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: 90ef4cd4-fd1e-4198-9a66-d133c660f646
⛔ Files ignored due to path filters (220)
go.sumis excluded by!**/*.sumstaging/api/go.sumis excluded by!**/*.sumstaging/operator-lifecycle-manager/go.sumis excluded by!**/*.sumstaging/operator-registry/go.sumis excluded by!**/*.sumvendor/github.com/containerd/containerd/version/version.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/docker/cli/AUTHORSis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-logr/logr/context_noslog.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-logr/logr/context_slog.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-logr/logr/funcr/funcr.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-logr/logr/funcr/slogsink.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-logr/logr/sloghandler.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-logr/logr/slogr.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-logr/logr/slogr/slogr.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-logr/logr/slogsink.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/cel-go/cel/env.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/cel-go/cel/folding.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/cel-go/cel/library.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/cel-go/cel/options.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/cel-go/cel/program.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/cel-go/cel/prompt.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/cel-go/checker/cost.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/cel-go/common/containers/container.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/cel-go/common/functions/functions.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/cel-go/common/runes/buffer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/cel-go/common/source.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/cel-go/common/types/timestamp.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/cel-go/common/types/unknown.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/cel-go/ext/BUILD.bazelis excluded by!**/vendor/**,!vendor/**vendor/github.com/google/cel-go/ext/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/google/cel-go/ext/bindings.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/cel-go/ext/encoders.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/cel-go/ext/lists.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/cel-go/ext/network.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/cel-go/ext/strings.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/cel-go/interpreter/BUILD.bazelis excluded by!**/vendor/**,!vendor/**vendor/github.com/google/cel-go/interpreter/activation.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/cel-go/interpreter/attributes.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/cel-go/interpreter/decorators.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/cel-go/interpreter/frame.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/cel-go/interpreter/interpretable.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/cel-go/interpreter/interpreter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/cel-go/interpreter/planner.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/cel-go/interpreter/runtimecost.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/cel-go/parser/unparser.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/grpc-ecosystem/grpc-health-probe/main.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/joelanford/ignore/.golangci.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/joelanford/ignore/ignore.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/flate/dict_decoder.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/flate/inflate.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/huff0/build_table.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/internal/snapref/decode.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/dict.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/enc_base.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/enc_best.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/enc_better.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/enc_dfast.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/enc_fast.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/enc_jobs.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/encoder.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/encoder_options.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/fse_decoder_amd64.sis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/fse_decoder_arm64.sis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/fse_decoder_asm.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/fse_decoder_generic.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/seqdec_amd64.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/seqdec_amd64.sis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/seqdec_arm64.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/seqdec_arm64.sis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/seqdec_asm.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/seqdec_generic.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/mattn/go-sqlite3/.coderabbit.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/mattn/go-sqlite3/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/mattn/go-sqlite3/callback.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/mattn/go-sqlite3/sqlite3-binding.cis excluded by!**/vendor/**,!vendor/**vendor/github.com/mattn/go-sqlite3/sqlite3-binding.his excluded by!**/vendor/**,!vendor/**vendor/github.com/mattn/go-sqlite3/sqlite3.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/mattn/go-sqlite3/sqlite3_load_extension.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/mattn/go-sqlite3/sqlite3_opt_preupdate_hook.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/mattn/go-sqlite3/sqlite3_opt_serialize.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/mattn/go-sqlite3/sqlite3_opt_vtable.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/operator-framework/api/pkg/manifests/bundleloader.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/operator-framework/api/pkg/manifests/packagemanifestloader.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/operator-framework/operator-lifecycle-manager/util/cpb/main.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/operator-framework/operator-registry/pkg/image/containersimageregistry/registry.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/operator-framework/operator-registry/pkg/lib/bundle/chartutil.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/operator-framework/operator-registry/pkg/lib/bundle/generate.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/operator-framework/operator-registry/pkg/lib/indexer/indexer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/operator-framework/operator-registry/pkg/sqlite/load.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/internal/github.com/golang/gddo/httputil/header/header.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/collectors/go_collector_go116.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/collectors/go_collector_latest.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/counter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/desc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/expvar_collector.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/gauge.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/go_collector_go116.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/go_collector_latest.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/histogram.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/internal/difflib.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/labels.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/metric.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/process_collector_darwin.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/process_collector_windows.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/promhttp/http.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/promhttp/instrument_client.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/promhttp/instrument_server.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/promhttp/option.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/registry.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/summary.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/timer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/vec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/wrap.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/Makefile.commonis excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/SECURITY.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/crypto.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/mountinfo.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/net_wireless.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/proc_cgroup.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/spiffe/go-spiffe/v2/bundle/jwtbundle/bundle.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/spiffe/go-spiffe/v2/bundle/spiffebundle/bundle.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/spiffe/go-spiffe/v2/bundle/spiffebundle/set.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/spiffe/go-spiffe/v2/exp/bundle/witbundle/bundle.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/spiffe/go-spiffe/v2/exp/bundle/witbundle/set.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/spiffe/go-spiffe/v2/exp/bundle/witbundle/source.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/spiffe/go-spiffe/v2/exp/svid/witsvid/source.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/spiffe/go-spiffe/v2/exp/svid/witsvid/svid.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/spiffe/go-spiffe/v2/proto/spiffe/workload/workload.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/spiffe/go-spiffe/v2/proto/spiffe/workload/workload.protois excluded by!**/vendor/**,!vendor/**vendor/github.com/spiffe/go-spiffe/v2/proto/spiffe/workload/workload_grpc.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/spiffe/go-spiffe/v2/spiffetls/tlsconfig/config.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/spiffe/go-spiffe/v2/svid/x509svid/svid.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/spiffe/go-spiffe/v2/workloadapi/client.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/spiffe/go-spiffe/v2/workloadapi/convenience.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/spiffe/go-spiffe/v2/workloadapi/option.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/spiffe/go-spiffe/v2/workloadapi/watcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/spiffe/go-spiffe/v2/workloadapi/witsource.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/bbolt/.gitattributesis excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/bbolt/.go-versionis excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/bbolt/.golangci.yamlis excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/bbolt/Makefileis excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/bbolt/OWNERSis excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/bbolt/README.mdis excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/bbolt/bucket.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/bbolt/code-of-conduct.mdis excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/bbolt/db.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/bbolt/errors/errors.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/bbolt/internal/common/page.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/bbolt/internal/freelist/hashmap.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/bbolt/internal/freelist/shared.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/bbolt/tx.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/bbolt/tx_check.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/crypto/openpgp/armor/armor.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/crypto/openpgp/errors/errors.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/crypto/openpgp/packet/packet.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/crypto/openpgp/read.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/crypto/openpgp/s2k/s2k.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/mod/modfile/read.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/mod/modfile/rule.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/http2/transport_wrap.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/idna/idna.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sync/semaphore/semaphore.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/cpu/parse.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/syscall_linux.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/syscall_linux_386.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/syscall_linux_amd64.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/syscall_linux_arm.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/syscall_linux_arm64.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/syscall_linux_loong64.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/syscall_linux_mips64x.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/syscall_linux_mipsx.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/syscall_linux_ppc.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/syscall_linux_ppc64x.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/syscall_linux_riscv64.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/syscall_linux_s390x.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/syscall_linux_sparc64.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zerrors_linux.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zsyscall_linux.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zsyscall_linux_386.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zsyscall_linux_amd64.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zsyscall_linux_arm.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zsyscall_linux_arm64.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zsyscall_linux_loong64.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zsyscall_linux_mips.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zsyscall_linux_mips64.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zsyscall_linux_mips64le.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zsyscall_linux_mipsle.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zsyscall_linux_ppc.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zsyscall_linux_ppc64.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zsyscall_linux_ppc64le.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zsyscall_linux_riscv64.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zsyscall_linux_s390x.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zsyscall_linux_sparc64.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/windows/security_windows.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/windows/syscall_windows.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/windows/types_windows.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/cases/context.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/cases/map.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/unicode/norm/forminfo.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/unicode/norm/iter.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/unicode/norm/normalize.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/packages/packages.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/gcimporter/iexport.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/gcimporter/iimport.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/imports/fix.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/imports/imports.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/stdlib/deps.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/stdlib/manifest.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/typesinternal/element.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/typesinternal/types.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/typesinternal/zerovalue.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/envconfig/envconfig.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/transport/controlbuf.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/transport/http2_client.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/transport/http2_server.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/version.gois excluded by!**/vendor/**,!vendor/**vendor/modules.txtis excluded by!**/vendor/**,!vendor/**vendor/oras.land/oras-go/v2/content/reader.gois excluded by!**/vendor/**,!vendor/**vendor/oras.land/oras-go/v2/errdef/errors.gois excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (38)
go.modmanifests/0000_50_olm_01-networkpolicies.yamlmicroshift-manifests/0000_50_olm_01-networkpolicies.yamlstaging/api/.github/workflows/go-verdiff.yamlstaging/api/.github/workflows/go.yamlstaging/api/.github/workflows/verify.ymlstaging/api/go.modstaging/api/pkg/manifests/bundleloader.gostaging/api/pkg/manifests/packagemanifestloader.gostaging/api/pkg/manifests/walkfunc_test.gostaging/operator-lifecycle-manager/.github/workflows/e2e-tests.ymlstaging/operator-lifecycle-manager/.github/workflows/goreleaser.yamlstaging/operator-lifecycle-manager/.github/workflows/sanity.yamlstaging/operator-lifecycle-manager/.github/workflows/unit.ymlstaging/operator-lifecycle-manager/deploy/chart/templates/0000_50_olm_01-networkpolicies.yamlstaging/operator-lifecycle-manager/go.modstaging/operator-lifecycle-manager/pkg/controller/operators/catalog/operator_test.gostaging/operator-lifecycle-manager/util/cpb/main.gostaging/operator-registry/.github/workflows/build.yamlstaging/operator-registry/.github/workflows/go-apidiff.yamlstaging/operator-registry/.github/workflows/go-verdiff.yamlstaging/operator-registry/.github/workflows/goreleaser.yamlstaging/operator-registry/.github/workflows/sanity.yamlstaging/operator-registry/.github/workflows/test.ymlstaging/operator-registry/.github/workflows/unit.yamlstaging/operator-registry/go.modstaging/operator-registry/pkg/image/containersimageregistry/registry.gostaging/operator-registry/pkg/image/containersimageregistry/registry_test.gostaging/operator-registry/pkg/lib/bundle/chartutil.gostaging/operator-registry/pkg/lib/bundle/generate.gostaging/operator-registry/pkg/lib/bundle/generate_test.gostaging/operator-registry/pkg/lib/bundle/utils_test.gostaging/operator-registry/pkg/lib/bundle/validate_test.gostaging/operator-registry/pkg/lib/indexer/indexer.gostaging/operator-registry/pkg/prettyunmarshaler/prettyunmarshaler_test.gostaging/operator-registry/pkg/sqlite/directory_test.gostaging/operator-registry/pkg/sqlite/load.gostaging/operator-registry/pkg/sqlite/substitutesfor_cycle_test.go
🚧 Files skipped from review as they are similar to previous changes (9)
- staging/operator-lifecycle-manager/.github/workflows/unit.yml
- staging/operator-lifecycle-manager/.github/workflows/goreleaser.yaml
- staging/operator-lifecycle-manager/.github/workflows/e2e-tests.yml
- staging/operator-lifecycle-manager/.github/workflows/sanity.yaml
- staging/operator-lifecycle-manager/util/cpb/main.go
- microshift-manifests/0000_50_olm_01-networkpolicies.yaml
- manifests/0000_50_olm_01-networkpolicies.yaml
- staging/operator-lifecycle-manager/deploy/chart/templates/0000_50_olm_01-networkpolicies.yaml
- staging/operator-lifecycle-manager/pkg/controller/operators/catalog/operator_test.go
| walkErr := errors.New("permission denied") | ||
|
|
||
| err := loader.LoadBundleWalkFunc("some/path", nil, walkErr) | ||
| require.ErrorIs(t, err, walkErr) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add diagnostic messages to the assertions.
Several require calls omit failure messages, making failures in these new cases harder to diagnose. Add concise context to the ErrorIs, Len, Error, Contains, and Mkdir assertions.
As per coding guidelines, assertions should include meaningful failure messages.
Also applies to: 25-25, 33-33, 46-47, 54-55, 63-63, 71-71, 80-80, 85-85, 94-94, 99-99
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@staging/api/pkg/manifests/walkfunc_test.go` at line 17, The assertions in
walkfunc tests lack diagnostic context. Update the affected require.ErrorIs,
require.Len, require.Error, require.Contains, and require.Mkdir calls to include
concise, meaningful failure messages describing the expected condition, while
preserving their existing assertions and behavior.
Source: Coding guidelines
| func TestLoadBundle_NonexistentDirectory(t *testing.T) { | ||
| dir := filepath.Join(t.TempDir(), "nonexistent") | ||
|
|
||
| loader := NewBundleLoader(dir) | ||
| err := loader.LoadBundle() | ||
| require.Error(t, err) | ||
| } | ||
|
|
||
| func TestLoadPackage_NonexistentDirectory(t *testing.T) { | ||
| dir := filepath.Join(t.TempDir(), "nonexistent") | ||
|
|
||
| loader := NewPackageManifestLoader(dir) | ||
| err := loader.LoadPackage() | ||
| require.Error(t, err) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert the specific walk failure, not just any error.
LoadBundle and LoadPackage can return unrelated validation errors, so require.Error alone does not prove that a nonexistent or inaccessible path error was propagated from filepath.Walk. Assert that the returned aggregate contains the expected walk-specific cause.
Also applies to: 74-100
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@staging/api/pkg/manifests/walkfunc_test.go` around lines 58 - 72, Strengthen
TestLoadBundle_NonexistentDirectory and TestLoadPackage_NonexistentDirectory by
asserting that each returned aggregate error contains the expected filesystem
walk failure from filepath.Walk, rather than only checking that an error exists.
Use the project’s existing aggregate-error inspection and path-not-found cause
symbols where available, preserving the current nonexistent-directory setup.
|
/retest
These are sig-builds and sig-network-edge (HAProxy router) tests — none |
|
/test e2e-gcp-ovn |
|
/lgtm |
|
/retitle OCPBUGS-76752, OCPBUGS-96749: Synchronize From Upstream Repositories The upstream moby updates fix these CVEs |
|
@openshift-bot: This pull request references Jira Issue OCPBUGS-76752, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. This pull request references Jira Issue OCPBUGS-96749, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
The bug has been updated to refer to the pull request using the external bug tracker. 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. |
|
/retitle OCPBUGS-96752, OCPBUGS-96749: Synchronize From Upstream Repositories Try again |
|
@openshift-bot: This pull request references Jira Issue OCPBUGS-96752, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
The bug has been updated to refer to the pull request using the external bug tracker. This pull request references Jira Issue OCPBUGS-96749, which is valid. 3 validation(s) were run on this bug
The bug has been updated to refer to the pull request using the external bug tracker. 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. |
|
/test e2e-aws-upgrade-ovn-signle-node |
|
/test e2e-aws-upgrade-ovn-single-node |
|
@openshift-bot: This pull request explicitly references no 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. |
|
/retest |
|
@openshift-bot: 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. |
The staging/ and vendor/ directories have been synchronized from the upstream repositories, pulling in the following commits:
This pull request is expected to merge without any human intervention. If tests are failing here, changes must land upstream to fix any issues so that future downstreaming efforts succeed.
/assign @openshift/openshift-team-operator-runtime