CORENET-7114: Add TLS profile support to the ReconcileOperConfig controller#3014
Open
tpantelis wants to merge 3 commits into
Open
CORENET-7114: Add TLS profile support to the ReconcileOperConfig controller#3014tpantelis wants to merge 3 commits into
tpantelis wants to merge 3 commits into
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 ✨ 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
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: tpantelis The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
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
Move environment variable reads from package-level initialization to NewHyperShiftConfig() function call. This allows environment variables to be set at test time rather than requiring them to be set before package initialization. Signed-off-by: Tom Pantelis <tompantelis@gmail.com>
The tests set up a controller-runtime Manager with a fake Cache implementation that bridges the informers with the controller reconcile queue. The tests use the watchers created by the fake Cache to inject object events to trigger reconciliation. Some tests inject various events to merely verify reconciliation is triggered by checking for any status condition added by the StatusManager initiated by the reconciliation logic. These test the various watchers added by the ReconcileOperConfig controller (motivated by the APIServer watcher that handles TLS profile changes). The tests use the existing FakeClient with some modifications: **Custom Informer Support:** - Added customInformers field to FakeClusterClient - Implemented Start() method to run custom informers - Implemented AddCustomInformer() to store informers for later startup **RESTMapper Improvements:** - Removed stub fakeRESTMapper implementation - Added real RESTMapper using scheme.Scheme.PrioritizedVersionsAllGroups() - Properly registers all known types for GVK/GVR mapping - RESTMapper() now returns functional mapper - Scheme() now returns scheme.Scheme instead of panicking **Server-Side Apply Support:** - Added applyPatchReactor() to handle ApplyPatchType patches - Intercepts Server-Side Apply operations in FakeDynamicClient - Creates objects if they don't exist (mimics real SSA behavior) - Updates existing objects with patch data Signed-off-by: Tom Pantelis <tompantelis@gmail.com>
tpantelis
force-pushed
the
tls_profile_support
branch
from
526901b to
55ffe0a
Compare
...to enable TLS profile compliance for the various component deployments rendered by the controller. 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 apiserver.config.openshift.io/cluster resource - Add APIServer watch in ReconcileOperConfig to trigger reconciliation when TLS profile or adherence policy changes. - Add helper function AddTLSInfoToRenderData() in pkg/network/tls.go to prepare TLS data for template rendering - 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
55ffe0a to
54295fd
Compare
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
2 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