CORENET-7114: Add OCP centralized TLS profile support#3014
Open
tpantelis wants to merge 2 commits into
Open
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 TLS profile types and render helpers, integrates TLS retrieval into Bootstrap, extends controller watches for APIServer/HostedCluster TLS changes, refactors MTU prober to use manifestPath and faster polling, enhances fake client/cache and status manager, and adds comprehensive operconfig and network tests. ChangesTLS Profile Support with Comprehensive Testing
🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 11❌ Failed checks (1 warning, 10 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Author
|
/cc @danwinship |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
pkg/client/fake/fake_client.go (1)
194-198: 💤 Low valueAll types are registered as namespace-scoped, which may cause issues with cluster-scoped resources.
The REST mapper adds all known types with
meta.RESTScopeNamespace, but cluster-scoped resources likeClusterOperator,Node,Infrastructure, etc., should usemeta.RESTScopeRoot. This could cause unexpected behavior if code paths rely on scope information from the mapper.Consider determining the correct scope per type, or at minimum, document this limitation.
💡 Potential fix to handle cluster-scoped types
for gvk := range scheme.Scheme.AllKnownTypes() { - restMapper.Add(gvk, meta.RESTScopeNamespace) + // Determine scope based on type - cluster-scoped types don't have namespace + // This is a simplification; a more complete solution would check API discovery + scope := meta.RESTScopeNamespace + if isClusterScoped(gvk) { + scope = meta.RESTScopeRoot + } + restMapper.Add(gvk, scope) }Where
isClusterScopedchecks known cluster-scoped kinds likeNode,Namespace,ClusterOperator,Infrastructure, etc.🤖 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 `@pkg/client/fake/fake_client.go` around lines 194 - 198, The REST mapper is incorrectly adding every known GVK with meta.RESTScopeNamespace; update the loop that iterates scheme.Scheme.AllKnownTypes() and call restMapper.Add(gvk, scope) where scope is decided per-GVK (use meta.RESTScopeRoot for cluster-scoped kinds like Node, Namespace, ClusterOperator, Infrastructure, etc., and meta.RESTScopeNamespace otherwise) — implement a small helper (e.g., isClusterScoped(gvk) or isClusterKind(kind)) to determine scope and apply it when calling restMapper.Add, or at minimum add an inline comment documenting this limitation if you cannot resolve scopes programmatically.pkg/controller/operconfig/controller_test.go (1)
312-321: 💤 Low value
FakeClockmixes real and stubbed time, which may cause subtle test issues.
Now()returns the actual current time whileSince()always returns 3 minutes regardless of the input. This inconsistency could cause confusing behavior if any code path computes elapsed time using both methods, e.g.,clock.Since(clock.Now())would return 3 minutes even if called immediately.Consider using a fully deterministic fake clock (e.g., from
k8s.io/utils/clock/testing) or documenting this intentional behavior.💡 Alternative using k8s.io/utils/clock/testing
+import clocktesting "k8s.io/utils/clock/testing" -type FakeClock struct { -} - -func (f FakeClock) Now() time.Time { - return time.Now() -} - -func (f FakeClock) Since(t time.Time) time.Duration { - return time.Minute * 3 -} // In newTestDriver: -t.statusManager = statusmanager.NewWithClock(t.fakeClient, clusterOperatorName, "", &FakeClock{}) +fakeClock := clocktesting.NewFakeClock(time.Now()) +// Advance past the degraded threshold +fakeClock.Step(3 * time.Minute) +t.statusManager = statusmanager.NewWithClock(t.fakeClient, clusterOperatorName, "", fakeClock)🤖 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 `@pkg/controller/operconfig/controller_test.go` around lines 312 - 321, FakeClock is inconsistent because Now() returns real time while Since(t time.Time) always returns 3 minutes, which can produce surprising results (e.g., clock.Since(clock.Now()) != 0); change FakeClock to be deterministic by storing a fixed base time field and implement Now() to return that base time and Since(t) to compute baseTime.Sub(t) (or switch to k8s.io/utils/clock/testing.FakeClock and use its Now/Since utilities); update usages/tests accordingly to either advance the fake clock explicitly or rely on the deterministic behavior of the k8s fake clock.
🤖 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 `@pkg/controller/fake/cache.go`:
- Around line 41-101: GetInformer currently checks f.informers under a read lock
then creates and inserts a new informer under a write lock, which allows a race
where another goroutine creates the informer between the RUnlock and Lock. Fix
by using a double-checked locking pattern in GetInformer: keep the initial RLock
check as-is, but if not found, build fakeWatcher and newInformer (no
side-effects), then acquire f.Lock(), re-check f.informers[objType]; if an
informer now exists return that, otherwise set f.watchers[objType] = fakeWatcher
and f.informers[objType] = newInformer and release the lock, then start
newInformer.Run(ctx.Done()). Ensure you reference the GetInformer function and
the f.informers / f.watchers maps and handle the fakeWatcher/newInformer
replacement logic accordingly.
In `@pkg/controller/operconfig/controller_hypershift_test.go`:
- Around line 196-198: The test currently only checks the annotation when it
exists, letting the case where it's missing pass silently; update the test to
explicitly assert that co.Annotations contains the key
names.RelatedClusterObjectsAnnotation (e.g., Expect/co.Annotations to have that
key) and then assert the value is not empty using the value at
co.Annotations[names.RelatedClusterObjectsAnnotation]; replace the conditional
"if annotation, exists := ..." pattern with two explicit expectations so absence
fails the test.
---
Nitpick comments:
In `@pkg/client/fake/fake_client.go`:
- Around line 194-198: The REST mapper is incorrectly adding every known GVK
with meta.RESTScopeNamespace; update the loop that iterates
scheme.Scheme.AllKnownTypes() and call restMapper.Add(gvk, scope) where scope is
decided per-GVK (use meta.RESTScopeRoot for cluster-scoped kinds like Node,
Namespace, ClusterOperator, Infrastructure, etc., and meta.RESTScopeNamespace
otherwise) — implement a small helper (e.g., isClusterScoped(gvk) or
isClusterKind(kind)) to determine scope and apply it when calling
restMapper.Add, or at minimum add an inline comment documenting this limitation
if you cannot resolve scopes programmatically.
In `@pkg/controller/operconfig/controller_test.go`:
- Around line 312-321: FakeClock is inconsistent because Now() returns real time
while Since(t time.Time) always returns 3 minutes, which can produce surprising
results (e.g., clock.Since(clock.Now()) != 0); change FakeClock to be
deterministic by storing a fixed base time field and implement Now() to return
that base time and Since(t) to compute baseTime.Sub(t) (or switch to
k8s.io/utils/clock/testing.FakeClock and use its Now/Since utilities); update
usages/tests accordingly to either advance the fake clock explicitly or rely on
the deterministic behavior of the k8s fake clock.
🪄 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: 95684223-4841-4ced-86e7-cddd68f08edb
⛔ Files ignored due to path filters (182)
go.sumis excluded by!**/*.sumvendor/github.com/go-task/slim-sprig/v3/.editorconfigis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/.gitattributesis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/.gitignoreis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/CHANGELOG.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/LICENSE.txtis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/Taskfile.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/crypto.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/date.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/defaults.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/dict.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/functions.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/list.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/network.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/numeric.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/reflect.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/regex.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/strings.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/url.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/.gitignoreis excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/CHANGELOG.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/CONTRIBUTING.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/Makefileis excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/RELEASING.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/config/deprecated.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/core_dsl.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/decorator_dsl.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/deprecated_dsl.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/formatter/colorable_others.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/formatter/colorable_windows.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/formatter/formatter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/automaxprocs.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/cgroup.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/cgroups.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/cgroups2.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/cpu_quota_linux.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/cpu_quota_unsupported.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/errors.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/mountpoint.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/runtime.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/subsys.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/build/build_command.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/command/abort.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/command/command.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/command/program.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/generators/boostrap_templates.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/generators/bootstrap_command.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/generators/generate_command.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/generators/generate_templates.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/generators/generators_common.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/internal/compile.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/internal/gocovmerge.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/internal/profiles_and_reports.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/internal/run.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/internal/test_suite.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/internal/utils.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/internal/verify_version.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/labels/labels_command.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/main.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/outline/ginkgo.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/outline/import.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/outline/outline.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/outline/outline_command.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/run/run_command.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/unfocus/unfocus_command.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/watch/delta.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/watch/delta_tracker.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/watch/dependencies.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/watch/package_hash.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/watch/package_hashes.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/watch/suite.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/watch/watch_command.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo_cli_dependencies.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo_t_dsl.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/around_node.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/counter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/failer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/focus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/global/init.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/group.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/interrupt_handler/interrupt_handler.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/interrupt_handler/sigquit_swallower_unix.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/interrupt_handler/sigquit_swallower_windows.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/node.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/ordering.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/output_interceptor.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/output_interceptor_unix.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/output_interceptor_wasm.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/output_interceptor_win.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/parallel_support/client_server.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/parallel_support/http_client.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/parallel_support/http_server.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/parallel_support/rpc_client.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/parallel_support/rpc_server.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/parallel_support/server_handler.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/progress_report.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/progress_report_bsd.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/progress_report_unix.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/progress_report_wasm.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/progress_report_win.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/progress_reporter_manager.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/report_entry.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/reporters/gojson.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/reporters/gojson_event_writer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/reporters/gojson_reporter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/spec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/spec_context.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/suite.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/testingtproxy/testing_t_proxy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/tree.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/writer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/reporters/default_reporter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/reporters/deprecated_reporter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/reporters/gojson_report.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/reporters/json_report.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/reporters/junit_report.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/reporters/reporter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/reporters/teamcity_report.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/reporting_dsl.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/table_dsl.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/around_node.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/code_location.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/config.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/deprecated_types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/deprecation_support.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/enum_support.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/errors.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/file_filter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/flags.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/label_filter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/report_entry.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/semver_filter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/version.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/gstruct/elements.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/gstruct/errors/nested_types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/gstruct/fields.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/gstruct/ignore.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/gstruct/keys.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/gstruct/pointer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/gstruct/types.gois excluded by!**/vendor/**,!vendor/**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/tls_adherence.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/cover/profile.gois excluded by!**/vendor/**,!vendor/**vendor/modules.txtis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/.gitignoreis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/.golangci.ymlis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/.gomodcheck.yamlis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/CONTRIBUTING.mdis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/FAQ.mdis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/Makefileis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/OWNERSis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/OWNERS_ALIASESis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/README.mdis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/RELEASE.mdis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/SECURITY_CONTACTSis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/TMP-LOGGING.mdis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/VERSIONING.mdis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/alias.gois excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/code-of-conduct.mdis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/doc.gois excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/builder/controller.gois excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/builder/doc.gois excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/builder/options.gois excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/builder/webhook.gois excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/client/config/config.gois excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/client/config/doc.gois excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/client/fake/versioned_tracker.gois excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/manager/signals/doc.gois excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/manager/signals/signal.gois excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/manager/signals/signal_posix.gois excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/manager/signals/signal_windows.gois excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/scheme/scheme.gois excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/webhook/admission/defaulter_custom.gois excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (22)
.golangci.yamlgo.modpkg/bootstrap/types.gopkg/client/fake/fake_client.gopkg/controller/fake/cache.gopkg/controller/operconfig/controller_applied_test.gopkg/controller/operconfig/controller_failures_test.gopkg/controller/operconfig/controller_hypershift_test.gopkg/controller/operconfig/controller_mtu_test.gopkg/controller/operconfig/controller_rendering_test.gopkg/controller/operconfig/controller_test.gopkg/controller/operconfig/controller_watch_test.gopkg/controller/operconfig/mtu_probe.gopkg/controller/operconfig/operconfig_controller.gopkg/controller/statusmanager/status_manager.gopkg/hypershift/hypershift.gopkg/network/bootstrap.gopkg/network/bootstrap_test.gopkg/network/network_suite_test.gopkg/network/render_test.gopkg/network/tls.gopkg/network/tls_test.go
tpantelis
force-pushed
the
tls_profile_support
branch
from
3a8ac96 to
d4d99b4
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
pkg/client/fake/fake_client.go (1)
195-198: 💤 Low valueAll types registered as namespace-scoped in RESTMapper.
The loop adds all known types with
meta.RESTScopeNamespace, but cluster-scoped resources likeNode,ClusterOperator, andInfrastructureshould usemeta.RESTScopeRoot. This could cause issues if tests rely on the mapper to determine resource scope.Consider using the scheme's scope information or maintaining a list of cluster-scoped types:
for gvk := range scheme.Scheme.AllKnownTypes() { - restMapper.Add(gvk, meta.RESTScopeNamespace) + // Simplified: most resources are namespaced, cluster-scoped ones + // would need explicit handling if scope-dependent behavior is tested + restMapper.Add(gvk, meta.RESTScopeNamespace) }If tests don't rely on scope behavior, this is acceptable for fake client usage.
🤖 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 `@pkg/client/fake/fake_client.go` around lines 195 - 198, The RESTMapper initialization currently registers every GVK as namespace-scoped (restMapper := meta.NewDefaultRESTMapper(...) and the loop over scheme.Scheme.AllKnownTypes()), which is wrong for cluster-scoped resources; update the loop to consult the scheme's scope info (or maintain a set of known cluster-scoped GVKs) and call restMapper.Add(gvk, meta.RESTScopeRoot) for cluster-scoped types (e.g., Node, ClusterOperator, Infrastructure) and meta.RESTScopeNamespace for namespace-scoped types so the mapper reflects correct scopes.pkg/controller/operconfig/operconfig_controller.go (1)
243-252: 💤 Low valueInconsistent fallback behavior on type assertion failures.
When
ObjectNewassertion fails (line 245-247), it returnsfalse(no reconcile). WhenObjectOldassertion fails (line 249-252), it returnstrue(triggers reconcile). This asymmetry could cause unexpected behavior.Consider making the fallback consistent:
oldObj, ok := evt.ObjectOld.(*uns.Unstructured) if !ok { - return true + return false }Or add logging to understand why the assertion would fail.
🤖 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 `@pkg/controller/operconfig/operconfig_controller.go` around lines 243 - 252, The UpdateFunc's type assertions for evt.ObjectNew and evt.ObjectOld are inconsistent: newObj failure returns false while oldObj failure returns true, which can trigger unexpected reconciles; make the fallback behavior consistent by choosing one policy (e.g., return false on both assertion failures) and apply it to both checks in UpdateFunc (evt.ObjectNew and evt.ObjectOld of type *uns.Unstructured), and optionally add a processLogger.Warn or similar log that includes the event type and which assertion failed to aid debugging.
🤖 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 `@pkg/network/bootstrap_test.go`:
- Line 141: Fix the typo in the test description used in the Ginkgo When call:
change the string passed to When(...) that currently reads "the the
HostedCluster CR exists" to "the HostedCluster CR exists" in
pkg/network/bootstrap_test.go so the test description is correct.
In `@pkg/network/tls.go`:
- Around line 72-74: The code unconditionally type-asserts apiServerConfig to
map[string]interface{} which can panic if spec.configuration.apiServer exists
but is not an object; update the logic in the TLS conversion path to first test
the concrete type (e.g., m, ok := apiServerConfig.(map[string]interface{})) and
return a descriptive error when the assertion fails before calling
runtime.DefaultUnstructuredConverter.FromUnstructured, ensuring apiServerSpec is
only populated from a verified map; reference apiServerConfig, apiServerSpec,
runtime.DefaultUnstructuredConverter.FromUnstructured and the
bootstrap.TLSProfile error return to locate the change.
---
Nitpick comments:
In `@pkg/client/fake/fake_client.go`:
- Around line 195-198: The RESTMapper initialization currently registers every
GVK as namespace-scoped (restMapper := meta.NewDefaultRESTMapper(...) and the
loop over scheme.Scheme.AllKnownTypes()), which is wrong for cluster-scoped
resources; update the loop to consult the scheme's scope info (or maintain a set
of known cluster-scoped GVKs) and call restMapper.Add(gvk, meta.RESTScopeRoot)
for cluster-scoped types (e.g., Node, ClusterOperator, Infrastructure) and
meta.RESTScopeNamespace for namespace-scoped types so the mapper reflects
correct scopes.
In `@pkg/controller/operconfig/operconfig_controller.go`:
- Around line 243-252: The UpdateFunc's type assertions for evt.ObjectNew and
evt.ObjectOld are inconsistent: newObj failure returns false while oldObj
failure returns true, which can trigger unexpected reconciles; make the fallback
behavior consistent by choosing one policy (e.g., return false on both assertion
failures) and apply it to both checks in UpdateFunc (evt.ObjectNew and
evt.ObjectOld of type *uns.Unstructured), and optionally add a
processLogger.Warn or similar log that includes the event type and which
assertion failed to aid debugging.
🪄 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: 7fc2c3ad-c2e9-4857-9af4-cb1774f3095a
⛔ Files ignored due to path filters (182)
go.sumis excluded by!**/*.sumvendor/github.com/go-task/slim-sprig/v3/.editorconfigis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/.gitattributesis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/.gitignoreis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/CHANGELOG.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/LICENSE.txtis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/Taskfile.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/crypto.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/date.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/defaults.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/dict.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/functions.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/list.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/network.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/numeric.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/reflect.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/regex.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/strings.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/url.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/.gitignoreis excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/CHANGELOG.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/CONTRIBUTING.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/Makefileis excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/RELEASING.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/config/deprecated.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/core_dsl.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/decorator_dsl.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/deprecated_dsl.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/formatter/colorable_others.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/formatter/colorable_windows.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/formatter/formatter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/automaxprocs.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/cgroup.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/cgroups.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/cgroups2.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/cpu_quota_linux.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/cpu_quota_unsupported.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/errors.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/mountpoint.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/runtime.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/subsys.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/build/build_command.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/command/abort.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/command/command.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/command/program.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/generators/boostrap_templates.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/generators/bootstrap_command.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/generators/generate_command.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/generators/generate_templates.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/generators/generators_common.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/internal/compile.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/internal/gocovmerge.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/internal/profiles_and_reports.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/internal/run.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/internal/test_suite.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/internal/utils.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/internal/verify_version.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/labels/labels_command.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/main.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/outline/ginkgo.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/outline/import.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/outline/outline.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/outline/outline_command.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/run/run_command.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/unfocus/unfocus_command.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/watch/delta.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/watch/delta_tracker.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/watch/dependencies.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/watch/package_hash.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/watch/package_hashes.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/watch/suite.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/watch/watch_command.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo_cli_dependencies.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo_t_dsl.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/around_node.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/counter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/failer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/focus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/global/init.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/group.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/interrupt_handler/interrupt_handler.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/interrupt_handler/sigquit_swallower_unix.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/interrupt_handler/sigquit_swallower_windows.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/node.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/ordering.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/output_interceptor.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/output_interceptor_unix.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/output_interceptor_wasm.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/output_interceptor_win.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/parallel_support/client_server.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/parallel_support/http_client.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/parallel_support/http_server.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/parallel_support/rpc_client.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/parallel_support/rpc_server.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/parallel_support/server_handler.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/progress_report.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/progress_report_bsd.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/progress_report_unix.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/progress_report_wasm.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/progress_report_win.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/progress_reporter_manager.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/report_entry.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/reporters/gojson.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/reporters/gojson_event_writer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/reporters/gojson_reporter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/spec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/spec_context.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/suite.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/testingtproxy/testing_t_proxy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/tree.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/writer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/reporters/default_reporter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/reporters/deprecated_reporter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/reporters/gojson_report.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/reporters/json_report.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/reporters/junit_report.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/reporters/reporter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/reporters/teamcity_report.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/reporting_dsl.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/table_dsl.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/around_node.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/code_location.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/config.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/deprecated_types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/deprecation_support.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/enum_support.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/errors.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/file_filter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/flags.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/label_filter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/report_entry.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/semver_filter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/version.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/gstruct/elements.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/gstruct/errors/nested_types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/gstruct/fields.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/gstruct/ignore.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/gstruct/keys.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/gstruct/pointer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/gstruct/types.gois excluded by!**/vendor/**,!vendor/**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/tls_adherence.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/cover/profile.gois excluded by!**/vendor/**,!vendor/**vendor/modules.txtis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/.gitignoreis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/.golangci.ymlis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/.gomodcheck.yamlis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/CONTRIBUTING.mdis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/FAQ.mdis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/Makefileis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/OWNERSis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/OWNERS_ALIASESis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/README.mdis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/RELEASE.mdis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/SECURITY_CONTACTSis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/TMP-LOGGING.mdis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/VERSIONING.mdis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/alias.gois excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/code-of-conduct.mdis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/doc.gois excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/builder/controller.gois excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/builder/doc.gois excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/builder/options.gois excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/builder/webhook.gois excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/client/config/config.gois excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/client/config/doc.gois excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/client/fake/versioned_tracker.gois excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/manager/signals/doc.gois excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/manager/signals/signal.gois excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/manager/signals/signal_posix.gois excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/manager/signals/signal_windows.gois excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/scheme/scheme.gois excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/webhook/admission/defaulter_custom.gois excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (26)
.golangci.yamlbindata/network/multus/multus.yamlbindata/network/ovn-kubernetes/common/008-script-lib.yamlbindata/network/ovn-kubernetes/common/error-cni.yamlbindata/network/ovn-kubernetes/common/pre-puller.yamlgo.modpkg/bootstrap/types.gopkg/client/fake/fake_client.gopkg/controller/fake/cache.gopkg/controller/operconfig/controller_applied_test.gopkg/controller/operconfig/controller_failures_test.gopkg/controller/operconfig/controller_hypershift_test.gopkg/controller/operconfig/controller_mtu_test.gopkg/controller/operconfig/controller_rendering_test.gopkg/controller/operconfig/controller_test.gopkg/controller/operconfig/controller_watch_test.gopkg/controller/operconfig/mtu_probe.gopkg/controller/operconfig/operconfig_controller.gopkg/controller/statusmanager/status_manager.gopkg/hypershift/hypershift.gopkg/network/bootstrap.gopkg/network/bootstrap_test.gopkg/network/network_suite_test.gopkg/network/render_test.gopkg/network/tls.gopkg/network/tls_test.go
✅ Files skipped from review due to trivial changes (2)
- .golangci.yaml
- pkg/network/tls_test.go
tpantelis
force-pushed
the
tls_profile_support
branch
from
d4d99b4 to
768ff86
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/controller/operconfig/mtu_probe.go (1)
45-46:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix incorrect platform name in Azure MTU log message.
At Line 45, the Azure path logs
"AWS cluster"; this makes debugging misleading.Suggested patch
- klog.Infof("AWS cluster, omitting MTU probing and using default of %d", azureMTU) + klog.Infof("Azure cluster, omitting MTU probing and using default of %d", azureMTU)🤖 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 `@pkg/controller/operconfig/mtu_probe.go` around lines 45 - 46, The log message incorrectly says "AWS cluster" when returning the Azure MTU; update the Infof call that references azureMTU to say "Azure cluster" instead of "AWS cluster" (look for the klog.Infof line that prints azureMTU in pkg/controller/operconfig/mtu_probe.go and adjust the string to "Azure cluster, omitting MTU probing and using default of %d").
🧹 Nitpick comments (5)
pkg/controller/operconfig/controller_test.go (1)
312-321: 💤 Low valueConsider documenting the intent of the hardcoded 3-minute duration in FakeClock.
FakeClock.Since()always returns 3 minutes, which exceeds thedegradedFailureDurationThreshold(2 minutes) inStatusManager. This causes the degraded condition to be set immediately without debouncing. While this is likely intentional for faster test execution, a comment would clarify this behavior.📝 Suggested documentation
type FakeClock struct { } func (f FakeClock) Now() time.Time { return time.Now() } +// Since always returns 3 minutes to exceed the degradedFailureDurationThreshold (2 minutes) +// in StatusManager, allowing tests to immediately observe degraded conditions without waiting. func (f FakeClock) Since(t time.Time) time.Duration { return time.Minute * 3 }🤖 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 `@pkg/controller/operconfig/controller_test.go` around lines 312 - 321, Add a short comment/docstring to FakeClock explaining why Since() returns a hardcoded 3-minute duration: state that returning 3 minutes intentionally exceeds StatusManager's degradedFailureDurationThreshold (2 minutes) so degraded condition triggers immediately for faster tests; reference the FakeClock type and its Since() method and mention the related constant degradedFailureDurationThreshold and StatusManager so future readers understand this deliberate test behavior.pkg/controller/operconfig/operconfig_controller.go (2)
243-252: ⚡ Quick winInconsistent handling of type assertion failures in UpdateFunc.
When
newObjassertion fails (line 245-247), the function returnsfalse(skip reconciliation). But whenoldObjassertion fails (line 249-252), it returnstrue(trigger reconciliation). This asymmetry could lead to unexpected behavior. Consider handling both consistently.♻️ Suggested fix for consistent behavior
UpdateFunc: func(evt event.UpdateEvent) bool { newObj, ok := evt.ObjectNew.(*uns.Unstructured) if !ok { - return false + return true // Reconcile on unexpected type to be safe } oldObj, ok := evt.ObjectOld.(*uns.Unstructured) if !ok { return true }Alternatively, return
falsefor both to skip on assertion failures:oldObj, ok := evt.ObjectOld.(*uns.Unstructured) if !ok { - return true + return false }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/controller/operconfig/operconfig_controller.go` around lines 243 - 252, The UpdateFunc currently treats failed type assertions asymmetrically: when evt.ObjectNew fails to cast to *uns.Unstructured it returns false, but when evt.ObjectOld fails it returns true; make them consistent by changing the oldObj assertion branch in UpdateFunc (the block checking evt.ObjectOld against *uns.Unstructured) to return false on failure, so both failed assertions skip reconciliation for UpdateFunc (refer to UpdateFunc, evt.ObjectNew, evt.ObjectOld, and uns.Unstructured).
254-261: 💤 Low valueErrors from unstructured field extraction are silently ignored.
The errors from
NestedFieldCopyandNestedStringare discarded. While these fields may be optional, ignoring errors could mask issues with malformed HostedCluster objects. Consider logging unexpected errors.🤖 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 `@pkg/controller/operconfig/operconfig_controller.go` around lines 254 - 261, The current change-detection logic uses uns.NestedFieldCopy and uns.NestedString to populate oldTLSProfile/ newTLSProfile and oldAdherence/ newAdherence but discards the returned errors; update the watcher comparator to check the error returns from NestedFieldCopy and NestedString, and log unexpected/malformed-field errors (including context like which object and field) before proceeding with the reflect.DeepEqual/ string comparison so malformed HostedCluster objects aren't silently ignored; keep the same semantics of only reconciling when the profile or adherence actually changed (use the same symbols oldTLSProfile, newTLSProfile, oldAdherence, newAdherence and reflect.DeepEqual).pkg/client/fake/fake_client.go (2)
195-198: 💤 Low valueAll types registered as namespace-scoped may cause issues for cluster-scoped resources.
The REST mapper registers all known types with
RESTScopeNamespace, but some resources (likeNamespace,Node,ClusterOperator) are cluster-scoped. This could cause incorrect behavior when the fake client needs to determine resource scope.Consider using
meta.RESTScopeRootfor cluster-scoped resources or using a more sophisticated mapper that respects the actual scope of each resource type.🤖 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 `@pkg/client/fake/fake_client.go` around lines 195 - 198, The REST mapper currently marks every GVK as namespace-scoped (restMapper.Add(gvk, meta.RESTScopeNamespace)), which is wrong for cluster-scoped resources; update the loop over scheme.Scheme.AllKnownTypes() so that for each gvk you detect cluster scope and call restMapper.Add(gvk, meta.RESTScopeRoot) for cluster-scoped types and meta.RESTScopeNamespace for namespaced ones. Implement detection by checking gvk.Kind/GroupVersion against a small whitelist/set (e.g., "Namespace", "Node", "ClusterOperator", etc.) or by consulting a helper that knows which kinds are namespaced, and update the code around restMapper/NewDefaultRESTMapper to use that decision when calling restMapper.Add.
149-159: 💤 Low valueSimplify redundant nil check after successful Get.
When
err == nil, the returnedobjfromTracker().Get()will be non-nil. The&& obj != nilcheck is redundant.♻️ Suggested simplification
// Try to get the existing object obj, err := dynClient.Tracker().Get(gvr, namespace, name) - if err == nil && obj != nil { + if err == nil { // Object exists, update it by replacing with the patch data // For Server-Side Apply, we just replace the whole object err = dynClient.Tracker().Update(gvr, unstructuredObj, namespace)🤖 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 `@pkg/client/fake/fake_client.go` around lines 149 - 159, The nil check on obj is redundant because dynClient.Tracker().Get(gvr, namespace, name) guarantees a non-nil obj when err == nil; simplify the conditional by removing "&& obj != nil" and only test "if err == nil" before calling dynClient.Tracker().Update(gvr, unstructuredObj, namespace) and returning the updated unstructuredObj and nil error, preserving the existing error handling around Update.
🤖 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.
Outside diff comments:
In `@pkg/controller/operconfig/mtu_probe.go`:
- Around line 45-46: The log message incorrectly says "AWS cluster" when
returning the Azure MTU; update the Infof call that references azureMTU to say
"Azure cluster" instead of "AWS cluster" (look for the klog.Infof line that
prints azureMTU in pkg/controller/operconfig/mtu_probe.go and adjust the string
to "Azure cluster, omitting MTU probing and using default of %d").
---
Nitpick comments:
In `@pkg/client/fake/fake_client.go`:
- Around line 195-198: The REST mapper currently marks every GVK as
namespace-scoped (restMapper.Add(gvk, meta.RESTScopeNamespace)), which is wrong
for cluster-scoped resources; update the loop over scheme.Scheme.AllKnownTypes()
so that for each gvk you detect cluster scope and call restMapper.Add(gvk,
meta.RESTScopeRoot) for cluster-scoped types and meta.RESTScopeNamespace for
namespaced ones. Implement detection by checking gvk.Kind/GroupVersion against a
small whitelist/set (e.g., "Namespace", "Node", "ClusterOperator", etc.) or by
consulting a helper that knows which kinds are namespaced, and update the code
around restMapper/NewDefaultRESTMapper to use that decision when calling
restMapper.Add.
- Around line 149-159: The nil check on obj is redundant because
dynClient.Tracker().Get(gvr, namespace, name) guarantees a non-nil obj when err
== nil; simplify the conditional by removing "&& obj != nil" and only test "if
err == nil" before calling dynClient.Tracker().Update(gvr, unstructuredObj,
namespace) and returning the updated unstructuredObj and nil error, preserving
the existing error handling around Update.
In `@pkg/controller/operconfig/controller_test.go`:
- Around line 312-321: Add a short comment/docstring to FakeClock explaining why
Since() returns a hardcoded 3-minute duration: state that returning 3 minutes
intentionally exceeds StatusManager's degradedFailureDurationThreshold (2
minutes) so degraded condition triggers immediately for faster tests; reference
the FakeClock type and its Since() method and mention the related constant
degradedFailureDurationThreshold and StatusManager so future readers understand
this deliberate test behavior.
In `@pkg/controller/operconfig/operconfig_controller.go`:
- Around line 243-252: The UpdateFunc currently treats failed type assertions
asymmetrically: when evt.ObjectNew fails to cast to *uns.Unstructured it returns
false, but when evt.ObjectOld fails it returns true; make them consistent by
changing the oldObj assertion branch in UpdateFunc (the block checking
evt.ObjectOld against *uns.Unstructured) to return false on failure, so both
failed assertions skip reconciliation for UpdateFunc (refer to UpdateFunc,
evt.ObjectNew, evt.ObjectOld, and uns.Unstructured).
- Around line 254-261: The current change-detection logic uses
uns.NestedFieldCopy and uns.NestedString to populate oldTLSProfile/
newTLSProfile and oldAdherence/ newAdherence but discards the returned errors;
update the watcher comparator to check the error returns from NestedFieldCopy
and NestedString, and log unexpected/malformed-field errors (including context
like which object and field) before proceeding with the reflect.DeepEqual/
string comparison so malformed HostedCluster objects aren't silently ignored;
keep the same semantics of only reconciling when the profile or adherence
actually changed (use the same symbols oldTLSProfile, newTLSProfile,
oldAdherence, newAdherence and reflect.DeepEqual).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: b89591c9-83fe-4138-8dfd-f9760ffbbd9b
⛔ Files ignored due to path filters (182)
go.sumis excluded by!**/*.sumvendor/github.com/go-task/slim-sprig/v3/.editorconfigis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/.gitattributesis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/.gitignoreis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/CHANGELOG.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/LICENSE.txtis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/Taskfile.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/crypto.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/date.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/defaults.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/dict.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/functions.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/list.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/network.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/numeric.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/reflect.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/regex.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/strings.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/url.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/.gitignoreis excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/CHANGELOG.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/CONTRIBUTING.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/Makefileis excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/RELEASING.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/config/deprecated.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/core_dsl.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/decorator_dsl.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/deprecated_dsl.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/formatter/colorable_others.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/formatter/colorable_windows.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/formatter/formatter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/automaxprocs.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/cgroup.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/cgroups.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/cgroups2.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/cpu_quota_linux.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/cpu_quota_unsupported.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/errors.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/mountpoint.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/runtime.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/subsys.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/build/build_command.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/command/abort.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/command/command.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/command/program.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/generators/boostrap_templates.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/generators/bootstrap_command.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/generators/generate_command.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/generators/generate_templates.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/generators/generators_common.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/internal/compile.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/internal/gocovmerge.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/internal/profiles_and_reports.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/internal/run.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/internal/test_suite.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/internal/utils.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/internal/verify_version.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/labels/labels_command.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/main.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/outline/ginkgo.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/outline/import.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/outline/outline.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/outline/outline_command.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/run/run_command.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/unfocus/unfocus_command.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/watch/delta.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/watch/delta_tracker.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/watch/dependencies.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/watch/package_hash.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/watch/package_hashes.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/watch/suite.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/watch/watch_command.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo_cli_dependencies.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo_t_dsl.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/around_node.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/counter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/failer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/focus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/global/init.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/group.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/interrupt_handler/interrupt_handler.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/interrupt_handler/sigquit_swallower_unix.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/interrupt_handler/sigquit_swallower_windows.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/node.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/ordering.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/output_interceptor.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/output_interceptor_unix.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/output_interceptor_wasm.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/output_interceptor_win.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/parallel_support/client_server.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/parallel_support/http_client.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/parallel_support/http_server.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/parallel_support/rpc_client.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/parallel_support/rpc_server.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/parallel_support/server_handler.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/progress_report.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/progress_report_bsd.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/progress_report_unix.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/progress_report_wasm.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/progress_report_win.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/progress_reporter_manager.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/report_entry.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/reporters/gojson.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/reporters/gojson_event_writer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/reporters/gojson_reporter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/spec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/spec_context.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/suite.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/testingtproxy/testing_t_proxy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/tree.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/writer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/reporters/default_reporter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/reporters/deprecated_reporter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/reporters/gojson_report.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/reporters/json_report.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/reporters/junit_report.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/reporters/reporter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/reporters/teamcity_report.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/reporting_dsl.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/table_dsl.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/around_node.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/code_location.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/config.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/deprecated_types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/deprecation_support.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/enum_support.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/errors.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/file_filter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/flags.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/label_filter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/report_entry.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/semver_filter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/version.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/gstruct/elements.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/gstruct/errors/nested_types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/gstruct/fields.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/gstruct/ignore.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/gstruct/keys.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/gstruct/pointer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/gstruct/types.gois excluded by!**/vendor/**,!vendor/**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/tls_adherence.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/cover/profile.gois excluded by!**/vendor/**,!vendor/**vendor/modules.txtis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/.gitignoreis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/.golangci.ymlis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/.gomodcheck.yamlis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/CONTRIBUTING.mdis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/FAQ.mdis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/Makefileis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/OWNERSis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/OWNERS_ALIASESis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/README.mdis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/RELEASE.mdis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/SECURITY_CONTACTSis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/TMP-LOGGING.mdis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/VERSIONING.mdis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/alias.gois excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/code-of-conduct.mdis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/doc.gois excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/builder/controller.gois excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/builder/doc.gois excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/builder/options.gois excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/builder/webhook.gois excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/client/config/config.gois excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/client/config/doc.gois excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/client/fake/versioned_tracker.gois excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/manager/signals/doc.gois excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/manager/signals/signal.gois excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/manager/signals/signal_posix.gois excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/manager/signals/signal_windows.gois excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/scheme/scheme.gois excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/webhook/admission/defaulter_custom.gois excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (22)
.golangci.yamlgo.modpkg/bootstrap/types.gopkg/client/fake/fake_client.gopkg/controller/fake/cache.gopkg/controller/operconfig/controller_applied_test.gopkg/controller/operconfig/controller_failures_test.gopkg/controller/operconfig/controller_hypershift_test.gopkg/controller/operconfig/controller_mtu_test.gopkg/controller/operconfig/controller_rendering_test.gopkg/controller/operconfig/controller_test.gopkg/controller/operconfig/controller_watch_test.gopkg/controller/operconfig/mtu_probe.gopkg/controller/operconfig/operconfig_controller.gopkg/controller/statusmanager/status_manager.gopkg/hypershift/hypershift.gopkg/network/bootstrap.gopkg/network/bootstrap_test.gopkg/network/network_suite_test.gopkg/network/render_test.gopkg/network/tls.gopkg/network/tls_test.go
✅ Files skipped from review due to trivial changes (1)
- .golangci.yaml
tpantelis
changed the title
openshift-ci-robot
added
the
jira/valid-reference
Contributor
|
@tpantelis: This pull request references CORENET-7114 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the sub-task to target the "5.0.0" version, but no target version was set. 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. |
tpantelis
force-pushed
the
tls_profile_support
branch
2 times, most recently
from
9c1f257 to
526901b
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pkg/controller/statusmanager/status_manager.go (1)
168-172: ⚡ Quick winMake
NewWithClocknil-safe by defaulting toclock.RealClock{}Current call sites use non-nil clocks (e.g.,
operconfig/controller_test.gopasses&FakeClock{}), but adding a nil fallback prevents a latent panic if future callers passnil.Suggested patch
func NewWithClock(client cnoclient.Client, name, cluster string, clock clock.PassiveClock) *StatusManager { status := New(client, name, cluster) - status.clock = clock + if clock == nil { + status.clock = clock.RealClock{} + } else { + status.clock = clock + } return status }🤖 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 `@pkg/controller/statusmanager/status_manager.go` around lines 168 - 172, NewWithClock currently assigns the provided clock directly which can panic if callers pass nil; update NewWithClock (in pkg/controller/statusmanager/status_manager.go) to default to a real clock when clock == nil by setting status.clock = clock if non-nil else status.clock = clock.RealClock{} (so StatusManager created via NewWithClock will always have a valid clock); keep the rest of the function (calling New(client, name, cluster) and returning status) unchanged.
🤖 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.
Nitpick comments:
In `@pkg/controller/statusmanager/status_manager.go`:
- Around line 168-172: NewWithClock currently assigns the provided clock
directly which can panic if callers pass nil; update NewWithClock (in
pkg/controller/statusmanager/status_manager.go) to default to a real clock when
clock == nil by setting status.clock = clock if non-nil else status.clock =
clock.RealClock{} (so StatusManager created via NewWithClock will always have a
valid clock); keep the rest of the function (calling New(client, name, cluster)
and returning status) unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: aaa1e01e-61a0-4805-a1b9-8ab9520d790c
⛔ Files ignored due to path filters (182)
go.sumis excluded by!**/*.sumvendor/github.com/go-task/slim-sprig/v3/.editorconfigis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/.gitattributesis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/.gitignoreis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/CHANGELOG.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/LICENSE.txtis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/Taskfile.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/crypto.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/date.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/defaults.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/dict.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/functions.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/list.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/network.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/numeric.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/reflect.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/regex.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/strings.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-task/slim-sprig/v3/url.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/.gitignoreis excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/CHANGELOG.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/CONTRIBUTING.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/Makefileis excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/RELEASING.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/config/deprecated.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/core_dsl.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/decorator_dsl.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/deprecated_dsl.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/formatter/colorable_others.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/formatter/colorable_windows.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/formatter/formatter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/automaxprocs.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/cgroup.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/cgroups.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/cgroups2.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/cpu_quota_linux.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/cpu_quota_unsupported.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/errors.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/mountpoint.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/runtime.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/subsys.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/build/build_command.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/command/abort.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/command/command.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/command/program.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/generators/boostrap_templates.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/generators/bootstrap_command.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/generators/generate_command.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/generators/generate_templates.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/generators/generators_common.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/internal/compile.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/internal/gocovmerge.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/internal/profiles_and_reports.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/internal/run.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/internal/test_suite.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/internal/utils.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/internal/verify_version.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/labels/labels_command.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/main.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/outline/ginkgo.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/outline/import.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/outline/outline.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/outline/outline_command.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/run/run_command.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/unfocus/unfocus_command.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/watch/delta.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/watch/delta_tracker.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/watch/dependencies.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/watch/package_hash.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/watch/package_hashes.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/watch/suite.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/watch/watch_command.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo_cli_dependencies.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo_t_dsl.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/around_node.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/counter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/failer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/focus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/global/init.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/group.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/interrupt_handler/interrupt_handler.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/interrupt_handler/sigquit_swallower_unix.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/interrupt_handler/sigquit_swallower_windows.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/node.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/ordering.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/output_interceptor.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/output_interceptor_unix.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/output_interceptor_wasm.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/output_interceptor_win.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/parallel_support/client_server.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/parallel_support/http_client.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/parallel_support/http_server.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/parallel_support/rpc_client.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/parallel_support/rpc_server.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/parallel_support/server_handler.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/progress_report.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/progress_report_bsd.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/progress_report_unix.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/progress_report_wasm.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/progress_report_win.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/progress_reporter_manager.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/report_entry.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/reporters/gojson.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/reporters/gojson_event_writer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/reporters/gojson_reporter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/spec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/spec_context.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/suite.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/testingtproxy/testing_t_proxy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/tree.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/writer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/reporters/default_reporter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/reporters/deprecated_reporter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/reporters/gojson_report.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/reporters/json_report.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/reporters/junit_report.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/reporters/reporter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/reporters/teamcity_report.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/reporting_dsl.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/table_dsl.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/around_node.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/code_location.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/config.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/deprecated_types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/deprecation_support.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/enum_support.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/errors.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/file_filter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/flags.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/label_filter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/report_entry.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/semver_filter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/version.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/gstruct/elements.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/gstruct/errors/nested_types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/gstruct/fields.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/gstruct/ignore.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/gstruct/keys.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/gstruct/pointer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/gstruct/types.gois excluded by!**/vendor/**,!vendor/**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/tls_adherence.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/cover/profile.gois excluded by!**/vendor/**,!vendor/**vendor/modules.txtis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/.gitignoreis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/.golangci.ymlis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/.gomodcheck.yamlis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/CONTRIBUTING.mdis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/FAQ.mdis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/Makefileis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/OWNERSis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/OWNERS_ALIASESis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/README.mdis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/RELEASE.mdis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/SECURITY_CONTACTSis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/TMP-LOGGING.mdis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/VERSIONING.mdis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/alias.gois excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/code-of-conduct.mdis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/doc.gois excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/builder/controller.gois excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/builder/doc.gois excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/builder/options.gois excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/builder/webhook.gois excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/client/config/config.gois excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/client/config/doc.gois excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/client/fake/versioned_tracker.gois excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/manager/signals/doc.gois excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/manager/signals/signal.gois excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/manager/signals/signal_posix.gois excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/manager/signals/signal_windows.gois excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/scheme/scheme.gois excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/webhook/admission/defaulter_custom.gois excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (22)
.golangci.yamlgo.modpkg/bootstrap/types.gopkg/client/fake/fake_client.gopkg/controller/fake/cache.gopkg/controller/operconfig/controller_applied_test.gopkg/controller/operconfig/controller_failures_test.gopkg/controller/operconfig/controller_hypershift_test.gopkg/controller/operconfig/controller_mtu_test.gopkg/controller/operconfig/controller_rendering_test.gopkg/controller/operconfig/controller_test.gopkg/controller/operconfig/controller_watch_test.gopkg/controller/operconfig/mtu_probe.gopkg/controller/operconfig/operconfig_controller.gopkg/controller/statusmanager/status_manager.gopkg/hypershift/hypershift.gopkg/network/bootstrap.gopkg/network/bootstrap_test.gopkg/network/network_suite_test.gopkg/network/render_test.gopkg/network/tls.gopkg/network/tls_test.go
🚧 Files skipped from review as they are similar to previous changes (20)
- .golangci.yaml
- pkg/network/network_suite_test.go
- pkg/network/bootstrap.go
- pkg/hypershift/hypershift.go
- go.mod
- pkg/controller/operconfig/controller_mtu_test.go
- pkg/controller/operconfig/controller_rendering_test.go
- pkg/network/tls.go
- pkg/controller/operconfig/controller_failures_test.go
- pkg/client/fake/fake_client.go
- pkg/controller/fake/cache.go
- pkg/controller/operconfig/mtu_probe.go
- pkg/network/bootstrap_test.go
- pkg/controller/operconfig/controller_hypershift_test.go
- pkg/network/render_test.go
- pkg/bootstrap/types.go
- pkg/network/tls_test.go
- pkg/controller/operconfig/operconfig_controller.go
- pkg/controller/operconfig/controller_test.go
- pkg/controller/operconfig/controller_applied_test.go
tpantelis
force-pushed
the
tls_profile_support
branch
2 times, most recently
from
55ffe0a to
54295fd
Compare
danwinship
reviewed
Jun 9, 2026
tpantelis
force-pushed
the
tls_profile_support
branch
2 times, most recently
from
31cb5e0 to
b654b02
Compare
Contributor
Author
|
I removed the oper config unit tests to simplify the PR and change the other unit tests I added in the |
tpantelis
force-pushed
the
tls_profile_support
branch
from
b654b02 to
10ddb5e
Compare
Contributor
|
You pushed a bunch of unrelated commits along with your branch... |
tpantelis
force-pushed
the
tls_profile_support
branch
from
10ddb5e to
fe3e0d6
Compare
Contributor
Author
|
@danwinship So it turns out CNO uses cluster-network-operator/cmd/cluster-network-operator/main.go Lines 49 to 51 in 6dc1804 which automatically sets up an HTTPS metrics server but with hard-coded TLS min version (TLS 1.2) and cipher suites, which, of course, isn't TLS profile compliant. I missed that before. It also does not (directly) provide a way to customize the TLS config, although it seems there is a clunky way to do it. Assuming that's true, we'll also need to watch for TLS profile changes and restart the process (there's no way to hot-reload the metrics server). Given that, the changes I made to the Other options are:
but I assume neither of those is acceptable. |
Contributor
I assume someone is going to have to fix that as part of all the 5.0 TLS work anyway though?
That's what we do for most components, and then add |
Contributor
Author
I asked about it on the #forum-ocp-tls-strict-obedience channel but no answer (yet). There's no open PR but I suspect if Anyway, there is a way to configure the TLS settings in
I assume that would entail adding a |
Contributor
Author
|
@danwinship I added e0a9cc3 to configure the TLS settings for the CNO metrics server. I moved the TLS profile watching from |
tpantelis
force-pushed
the
tls_profile_support
branch
2 times, most recently
from
2a9a463 to
2985d92
Compare
Contributor
Author
Done |
tpantelis
changed the title
Contributor
|
/verified by @weliang |
Contributor
|
@danwinship: This PR has been marked as verified by 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
verified
Contributor
|
@danwinship: Overrode contexts on behalf of danwinship: ci/prow/e2e-aws-ovn-upgrade-ipsec 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 kubernetes-sigs/prow repository. |
Contributor
Contributor
|
/override ci/prow/e2e-aws-ovn-upgrade-ipsec |
Contributor
|
/override ci/prow/e2e-metal-ipi-ovn-ipv6-ipsec ci/prow/e2e-ovn-ipsec-step-registry ci/prow/e2e-aws-ovn-upgrade-ipsec |
Contributor
|
@danwinship: Overrode contexts on behalf of danwinship: ci/prow/e2e-aws-ovn-upgrade-ipsec, ci/prow/e2e-metal-ipi-ovn-ipv6-ipsec, ci/prow/e2e-ovn-ipsec-step-registry 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 kubernetes-sigs/prow repository. |
Contributor
|
/override ci/prow/e2e-metal-ipi-ovn-ipv6-ipsec |
Contributor
|
@danwinship: Overrode contexts on behalf of danwinship: ci/prow/e2e-metal-ipi-ovn-ipv6-ipsec 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 kubernetes-sigs/prow repository. |
Contributor
|
@tpantelis serial test is failing because you added a new host-network listening socket that doesn't have a Service pointing to it... |
...to enable TLS profile compliance for the various component
deployments rendered by the ReconcileOperConfig controller and
for the CNO metrics server.
The CNO metrics server (port 9104) was using default TLS settings
but now support honoring the cluster TLS profile via two
coordinated mechanisms:
1. Apply TLS profile at startup: main.go hooks into the controllercmd
startup to configure TLS for metrics server. It obtians the
cluster's TLS profile and applies the TLS minVersion and
cipherSuites to the metrics server ServingInfo.
2. Restart on profile change: A new pkg/controller/tls controller
watches for TLS profile or adherence policy changes. When detected,
it calls the cancel function passed from main.go, triggering a
graceful restart via context cancellation.
Other changes:
- Add TLSProfile type to bootstrap.BootstrapResult to capture cluster
TLS configuration and adherence policy
- Implement GetTLSProfile() in pkg/network/bootstrap.go to fetch the TLS
settings from the Openshift APIServer resource or,
in HyperShift environments, the HostedControlPlane resource.
- Add helper function addTLSInfoToRenderData() in pkg/network/tls.go
to prepare TLS data for template rendering
- Add APIServerSpec field to the HostedControlPlane struct in hypershift
package
- Add GetHostedControlPlane() function in hypershift package
- Update dependencies: controller-runtime-common for TLS helpers and
library-go for ShouldHonorClusterTLSProfile()
Signed-off-by: Tom Pantelis <tompantelis@gmail.com>
tpantelis
force-pushed
the
tls_profile_support
branch
from
046d565 to
d11954f
Compare
openshift-ci-robot
removed
the
verified
Contributor
Author
So the refactoring to override the default Fixed by extracting and applying the PS: I think we should consider migrating to the controller runtime API in the next release instead of using library-go's controllercmd. |
Contributor
yeah, CNO needs a lot of refactoring... |
Contributor
|
/override ci/prow/e2e-metal-ipi-ovn-ipv6-ipsec |
Contributor
|
/verified by @weliang |
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: danwinship, tpantelis 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 |
Contributor
|
@danwinship: Overrode contexts on behalf of danwinship: ci/prow/e2e-metal-ipi-ovn-ipv6-ipsec 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 kubernetes-sigs/prow repository. |
openshift-ci-robot
added
the
verified
Contributor
|
@danwinship: This PR has been marked as verified by 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. |
Contributor
|
/retest-required |
Contributor
|
@danwinship: Overrode contexts on behalf of danwinship: ci/prow/e2e-aws-ovn-upgrade-ipsec, ci/prow/e2e-ovn-ipsec-step-registry 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 kubernetes-sigs/prow repository. |
Contributor
|
@tpantelis: The following tests 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. |
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
4 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.
See commits for details.
Summary by CodeRabbit
New Features
Improvements
Tests
Chores