Skip to content

CNF-19617, CNF-21768, CNF-21832, CNF-22018, OCPBUGS-59883, OCPBUGS-66407: Sync from upstream (07-Apr-2026)#692

Draft
nocturnalastro wants to merge 21 commits intoopenshift:mainfrom
nocturnalastro:upstream-sync-2026-04-07-rebase
Draft

CNF-19617, CNF-21768, CNF-21832, CNF-22018, OCPBUGS-59883, OCPBUGS-66407: Sync from upstream (07-Apr-2026)#692
nocturnalastro wants to merge 21 commits intoopenshift:mainfrom
nocturnalastro:upstream-sync-2026-04-07-rebase

Conversation

@nocturnalastro
Copy link
Copy Markdown
Contributor

@nocturnalastro nocturnalastro commented Apr 7, 2026

Upstream PRs included

  • #206 CNF-21768: Add TLS adherence support (CNF-21768)
  • #203 update OWNERS
  • #202 Upgrade to Go 1.25 and update dependencies
  • #201 Expose system-level and base board hardware details in NodePtpDevice Status (CNF-21832)
  • #200 Update Dockerfile builder image
  • #198 Update Dockerfile.krp to use Go version 1.25.7 for the builder stage
  • #197 CNF-19617: Add test coverage for clockClass verification when locking PTP source (CNF-19617,OCPBUGS-59883,OCPBUGS-66407)
  • #194 Set tls-profiles feature annotation to true in CSV
  • #192 Add must-gather collection to CI pipeline
  • #185 PTP CI: Add BC clock class recovery test for upstream link outage (CNF-22018)

Skipped PRs

  • #195 Configure AWS timeout to 2 hours for long jobs — all changed files are upstream-only (not present downstream)

jzding and others added 21 commits March 11, 2026 19:37
The operator now honors the cluster-wide TLS security profile from the
APIServer CR, so declare this capability via the OLM feature annotation.

Signed-off-by: Jack Ding <jackding@gmail.com>

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Move builder image to non-docker image so that we do not get hit with pull limits
Set tls-profiles feature annotation to true in CSV
Update Dockerfile.krp to use Go version 1.25.7 for the builder stage
This is needed for TLSAdherence support.

- Upgrade from Go 1.24 to Go 1.25.0
- Update openshift/api to v0.0.0-20260318185450-1f2fa3f09f4e
- Update openshift/library-go to v0.0.0-20260318142011-72bf34f474bc
- Update openshift/controller-runtime-common to v0.0.0-20260318085703-1812aed6dbd2
- Upgrade sigs.k8s.io/controller-runtime from v0.22.5 to v0.23.3
- Upgrade k8s.io dependencies from v0.34.3 to v0.35.2
- Update l2discovery-lib from v0.0.21 to v0.1.0
- Update l2discovery image in CI test
- Fix webhook registrations for controller-runtime v0.23 generic API

Signed-off-by: Jack Ding <jackding@gmail.com>
Upgrade to Go 1.25 and update dependencies
…-locked-after-degrading

PTP CI: Add BC clock class recovery test for upstream link outage
CNF-19617: Add test coverage for clockClass verification when locking PTP source
Read the tlsAdherence policy from the APIServer CR and use
ShouldHonorClusterTLSProfile to conditionally apply the cluster
TLS profile. In Legacy mode (default), Go TLS defaults are used.
In Strict mode, the cluster-wide TLS profile is enforced on the
operator's webhook/metrics servers and on the daemon's
kube-rbac-proxy.

The SecurityProfileWatcher now also monitors adherence policy
changes and triggers a graceful restart when the policy changes.

Signed-off-by: Jack Ding <jackding@gmail.com>
Signed-off-by: Jack Ding <jackding@gmail.com>
Expose system-level and base board hardware details in NodePtpDevice Status
Replace the separate TLSAdherencePolicy field on the reconciler
with a nil *TLSProfileSpec pointer pattern. When the pointer is
nil, legacy hardcoded ciphers are used. When non-nil, the cluster
TLS profile is applied. The adherence decision is made once in
main.go and expressed through the pointer value.

Signed-off-by: Jack Ding <jackding@gmail.com>
CNF-21768: Add TLS adherence support
@openshift-ci-robot openshift-ci-robot added jira/severity-important Referenced Jira bug's severity is important for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. labels Apr 7, 2026
@openshift-ci-robot
Copy link
Copy Markdown
Contributor

openshift-ci-robot commented Apr 7, 2026

@nocturnalastro: This pull request references CNF-19617 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 story to target the "4.22.0" version, but no target version was set.

This pull request references CNF-21768 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 story to target the "4.22.0" version, but no target version was set.

This pull request references CNF-21832 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 story to target the "4.22.0" version, but no target version was set.

This pull request references CNF-22018 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 story to target the "4.22.0" version, but no target version was set.

This pull request references Jira Issue OCPBUGS-59883, which is invalid:

  • expected the bug to be open, but it isn't
  • expected the bug to target either version "4.22." or "openshift-4.22.", but it targets "4.18.z" instead
  • expected the bug to be in one of the following states: NEW, ASSIGNED, POST, but it is Closed (Done) instead

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

This pull request references Jira Issue OCPBUGS-66407, which is invalid:

  • expected the bug to be open, but it isn't
  • expected the bug to target either version "4.22." or "openshift-4.22.", but it targets "4.20.z" instead
  • expected the bug to be in one of the following states: NEW, ASSIGNED, POST, but it is Closed (Done) instead

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

Upstream PRs included

  • #206 CNF-21768: Add TLS adherence support (CNF-21768)
  • #203 update OWNERS
  • #202 Upgrade to Go 1.25 and update dependencies
  • #201 Expose system-level and base board hardware details in NodePtpDevice Status (CNF-21832)
  • #200 Update Dockerfile builder image
  • #198 Update Dockerfile.krp to use Go version 1.25.7 for the builder stage
  • #197 CNF-19617: Add test coverage for clockClass verification when locking PTP source (CNF-19617,OCPBUGS-59883,OCPBUGS-66407)
  • #194 Set tls-profiles feature annotation to true in CSV
  • #192 Add must-gather collection to CI pipeline
  • #185 PTP CI: Add BC clock class recovery test for upstream link outage (CNF-22018)

Skipped PRs

  • #195 Configure AWS timeout to 2 hours for long jobs — all changed files are upstream-only (not present downstream)

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@openshift-ci-robot
Copy link
Copy Markdown
Contributor

openshift-ci-robot commented Apr 7, 2026

@nocturnalastro: This pull request references CNF-19617 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 story to target the "4.22.0" version, but no target version was set.

This pull request references CNF-21768 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 story to target the "4.22.0" version, but no target version was set.

This pull request references CNF-21832 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 story to target the "4.22.0" version, but no target version was set.

This pull request references CNF-22018 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 story to target the "4.22.0" version, but no target version was set.

This pull request references Jira Issue OCPBUGS-59883, which is invalid:

  • expected the bug to be open, but it isn't
  • expected the bug to target either version "4.22." or "openshift-4.22.", but it targets "4.18.z" instead
  • expected the bug to be in one of the following states: NEW, ASSIGNED, POST, but it is Closed (Done) instead

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

This pull request references Jira Issue OCPBUGS-66407, which is invalid:

  • expected the bug to be open, but it isn't
  • expected the bug to target either version "4.22." or "openshift-4.22.", but it targets "4.20.z" instead
  • expected the bug to be in one of the following states: NEW, ASSIGNED, POST, but it is Closed (Done) instead

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

Details

In response to this:

Upstream PRs included

  • #206 CNF-21768: Add TLS adherence support (CNF-21768)
  • #203 update OWNERS
  • #202 Upgrade to Go 1.25 and update dependencies
  • #201 Expose system-level and base board hardware details in NodePtpDevice Status (CNF-21832)
  • #200 Update Dockerfile builder image
  • #198 Update Dockerfile.krp to use Go version 1.25.7 for the builder stage
  • #197 CNF-19617: Add test coverage for clockClass verification when locking PTP source (CNF-19617,OCPBUGS-59883,OCPBUGS-66407)
  • #194 Set tls-profiles feature annotation to true in CSV
  • #192 Add must-gather collection to CI pipeline
  • #185 PTP CI: Add BC clock class recovery test for upstream link outage (CNF-22018)

Skipped PRs

  • #195 Configure AWS timeout to 2 hours for long jobs — all changed files are upstream-only (not present downstream)

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.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 7, 2026

Walkthrough

PR introduces PTP operator enhancements including new API types for system/baseboard information, updated Go toolchain (1.25), TLS profile support with legacy fallback, boundary clock conformance tests, and dependency upgrades. Webhook and CRD schemas updated accordingly.

Changes

Cohort / File(s) Summary
Docker Base Image Updates
Dockerfile, ptp-tools/Dockerfile.*
Updated Go toolchain from 1.24.x to 1.25.x; main Dockerfile changed registry to registry.access.redhat.com/ubi9/go-toolset:1.25; builder stage now runs as root user.
API Type Definitions
api/v1/nodeptpdevice_types.go, api/v1/zz_generated.deepcopy.go
Added new types SystemInfo and BaseBoardInfo with SMBIOS/DMI fields; extended NodePtpDeviceStatus with optional pointers to these types; generated deep-copy methods for new types.
Webhook Configuration
api/v1/ptpconfig_webhook.go, api/v1/ptpoperatorconfig_webhook.go
Changed webhook builder pattern from .For(r).WithValidator() to .NewWebhookManagedBy(mgr, r).WithCustomValidator() for both PtpConfig and PtpOperatorConfig validators.
CRD and Manifest Schemas
config/crd/bases/ptp.openshift.io_nodeptpdevices.yaml, bundle/manifests/ptp.openshift.io_nodeptpdevices.yaml, manifests/stable/ptp.openshift.io_nodeptpdevices.yaml
Extended NodePtpDeviceStatus schema with baseBoardInfo (4 string properties) and systemInfo (6 string properties) objects; consistent across all CRD/manifest files.
ClusterServiceVersion Metadata
bundle/manifests/ptp-operator.clusterserviceversion.yaml, manifests/stable/ptp-operator.clusterserviceversion.yaml, config/manifests/bases/ptp-operator.clusterserviceversion.yaml
Updated createdAt timestamp; changed features.operators.openshift.io/tls-profiles from "false" to "true" to advertise TLS profile support.
Monitoring Configuration
config/prometheus/monitor.yaml, bindata/linuxptp/ptp-daemon.yaml
Added client certificate/key paths (certFile, keyFile) to ServiceMonitor TLS config; extended kube-rbac-proxy args with conditional TLS cipher suites and min version templates.
TLS Profile Implementation
controllers/ptpoperatorconfig_controller.go, controllers/tls_profile_test.go, controllers/tls_watcher_test.go
Changed TLSProfileSpec from value to pointer to support nil/legacy mode; added setTLSTemplateData() helper to conditionally use profile or fallback to legacyCipherSuites; added tests for both profile and legacy paths.
Test Helpers and Utilities
test/pkg/ptptesthelper/ptptesthelper.go, test/pkg/testconfig/testconfig.go
Extended PortEngine with TurnAllPortsDown() and port state-transition methods; added NICInfo struct and functions for clock class discovery/verification via PMC and metrics; updated L2_DISCOVERY_IMAGE to v15.
Boundary Clock Conformance Tests
test/conformance/serial/ptp.go
Added BC clock class verification tests with gmCapable=1 configuration; implemented setupBCClockClassEvents() and verifyClockClassViaEvent() helpers; added upstream link outage recovery test.
OWNERS File
OWNERS
Removed top-level reviewers section; updated approvers by removing aneeshkp, adding edcdavid and sebsoto; retained existing approvers.
Script and Build Automation
scripts/k8s-start.sh, scripts/run-on-vm.sh, scripts/run-tests.sh, must-gather/collection-scripts/gather, ptp-tools/Dockerfile.debug, ptp-tools/Makefile
Added PriorityClass setup in k8s-start.sh; extended test scripts with --must-gather-image and --debug-image flags; added must-gather integration with EXIT trap; created new debug Dockerfile; added debug to Makefile VALUES.
Dependency and Module Updates
go.mod, main.go
Updated Go from 1.24.0 to 1.25.0; upgraded k8s.io modules from v0.34.3 to v0.35.2, controller-runtime to v0.23.3; added fetchTLSConfig() to fetch both TLS profile and adherence policy; extended watcher with OnAdherencePolicyChange callback.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

❤️ Share

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

@openshift-ci openshift-ci bot requested review from aneeshkp and vitus133 April 7, 2026 21:16
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci bot commented Apr 7, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: nocturnalastro

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

The pull request process is described here

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

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 7, 2026
@nocturnalastro nocturnalastro marked this pull request as draft April 7, 2026 21:22
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 7, 2026
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

🧹 Nitpick comments (4)
ptp-tools/Dockerfile.ptpop (1)

2-2: Use --no-install-recommends for the apt-get install command.

The current command pulls recommended packages unnecessarily, increasing build time and image size. This is inconsistent with the rest of the codebase.

Proposed change
-RUN apt-get update && apt-get install -y binutils-gold && rm -rf /var/lib/apt/lists/*
+RUN apt-get update && apt-get install -y --no-install-recommends binutils-gold && rm -rf /var/lib/apt/lists/*
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@ptp-tools/Dockerfile.ptpop` at line 2, Update the RUN instruction that
installs binutils-gold to avoid pulling recommended packages: change the apt-get
install invocation in the Dockerfile.ptpop RUN line to include
--no-install-recommends (i.e., apt-get update && apt-get install -y
--no-install-recommends binutils-gold && rm -rf /var/lib/apt/lists/*) so the
image build is smaller and consistent with the rest of the codebase.
controllers/tls_watcher_test.go (1)

40-50: Consider adding an assertion for TLSCipherSuites in the Modern profile test.

The comment on line 49 notes that TLS 1.3 ciphers aren't configurable in Go, but no assertion verifies the actual TLSCipherSuites value. Adding an assertion (even for an empty string) would document the expected behavior.

💡 Suggested addition
 func TestSetTLSTemplateData_ModernProfile(t *testing.T) {
 	profile := *configv1.TLSProfiles[configv1.TLSProfileModernType]
 	r := &PtpOperatorConfigReconciler{
 		TLSProfileSpec: &profile,
 	}
 	data := render.MakeRenderData()
 	r.setTLSTemplateData(&data)

 	assert.Equal(t, "VersionTLS13", data.Data["TLSMinVersion"])
 	// TLS 1.3 ciphers are not configurable in Go, so cipher list may be empty
+	expectedCiphers := strings.Join(libgocrypto.OpenSSLToIANACipherSuites(profile.Ciphers), ",")
+	assert.Equal(t, expectedCiphers, data.Data["TLSCipherSuites"],
+		"TLSCipherSuites should match converted profile ciphers (may be empty for TLS 1.3)")
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@controllers/tls_watcher_test.go` around lines 40 - 50, Add an explicit
assertion in TestSetTLSTemplateData_ModernProfile to verify the TLSCipherSuites
value produced by setTLSTemplateData: locate the test function
TestSetTLSTemplateData_ModernProfile, call r.setTLSTemplateData(&data) as
already done, then assert the expected value for data.Data["TLSCipherSuites"]
(e.g., assert.Equal(t, "", data.Data["TLSCipherSuites"])) to document that TLS
1.3 cipher suites are not configurable in Go.
controllers/tls_profile_test.go (1)

128-174: Consider referencing legacyCipherSuites constant instead of duplicating the cipher list.

The hardcoded cipher string on lines 133-137 and 168-172 duplicates the legacyCipherSuites constant from the controller. If the constant changes, these tests won't catch the drift.

💡 Suggested improvement
 func TestTLSProfileTemplateRendering_LegacyAdherence(t *testing.T) {
 	// When TLS adherence is legacy, kube-rbac-proxy should use the hardcoded
 	// cipher suites that were in place before cluster TLS profile support.
 	data := makeTestRenderData()
 	data.Data["TLSMinVersion"] = ""
-	data.Data["TLSCipherSuites"] = "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256," +
-		"TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256," +
-		"TLS_RSA_WITH_AES_128_CBC_SHA256," +
-		"TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256," +
-		"TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256"
+	data.Data["TLSCipherSuites"] = legacyCipherSuites
 
 	// ... assertions ...
 
-	assert.Contains(t, rbacProxyArgs,
-		"--tls-cipher-suites=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,"+
-			"TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,"+
-			"TLS_RSA_WITH_AES_128_CBC_SHA256,"+
-			"TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256,"+
-			"TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256")
+	assert.Contains(t, rbacProxyArgs, "--tls-cipher-suites="+legacyCipherSuites)
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@controllers/tls_profile_test.go` around lines 128 - 174, The test
TestTLSProfileTemplateRendering_LegacyAdherence duplicates the cipher list;
replace the hardcoded cipher string in the test (both where
data.Data["TLSCipherSuites"] is set and the assert.Contains expected value) with
a reference to the controller constant legacyCipherSuites so the test uses the
single source of truth; update imports if needed to access legacyCipherSuites
from the controller package and ensure the test assembles rbacProxyArgs
comparison against that constant.
api/v1/ptpconfig_webhook.go (1)

59-61: Consider migrating to WithValidator instead of the deprecated WithCustomValidator.

Per controller-runtime v0.23, WithCustomValidator is deprecated in favor of WithValidator. To migrate, update ptpConfigValidator methods to accept typed *PtpConfig parameters instead of runtime.Object:

func (v *ptpConfigValidator) ValidateCreate(ctx context.Context, obj *PtpConfig) (admission.Warnings, error)
func (v *ptpConfigValidator) ValidateUpdate(ctx context.Context, oldObj, newObj *PtpConfig) (admission.Warnings, error)
func (v *ptpConfigValidator) ValidateDelete(ctx context.Context, obj *PtpConfig) (admission.Warnings, error)

Then replace WithCustomValidator(&ptpConfigValidator{}) with WithValidator(&ptpConfigValidator{}). This eliminates type casts, improves type safety, and aligns with current controller-runtime practices.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@api/v1/ptpconfig_webhook.go` around lines 59 - 61, Replace the deprecated
WithCustomValidator usage and update ptpConfigValidator to the typed validator
API: change the call ctrl.NewWebhookManagedBy(mgr,
r).WithCustomValidator(&ptpConfigValidator{}).Complete() to use
WithValidator(&ptpConfigValidator{}), and modify ptpConfigValidator's methods to
the typed signatures ValidateCreate(ctx context.Context, obj *PtpConfig)
(admission.Warnings, error), ValidateUpdate(ctx context.Context, oldObj, newObj
*PtpConfig) (admission.Warnings, error) and ValidateDelete(ctx context.Context,
obj *PtpConfig) (admission.Warnings, error), removing runtime.Object casts and
returning the same behavior with the new typed parameters.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@Dockerfile`:
- Around line 1-3: The Dockerfile currently sets USER root in the builder stage
(FROM registry.access.redhat.com/ubi9/go-toolset:1.25 AS builder / USER root)
and never switches to a non-root user for the final runtime image; update the
Dockerfile to create or use a non-root user/group in the build (or final) stage
and add a USER <nonroot> directive in the runtime stage so the shipped container
does not run as root; ensure any runtime-owned files or directories are chowned
to that user (e.g., during the builder stage or in a dedicated setup step) and
reference the created username in the final USER instruction.

In `@main.go`:
- Around line 90-117: fetchTLSConfig() can return a sentinel "missing-policy"
result that should be treated as the non-OpenShift fallback, but other errors
(transient/RBAC/read failures) must not be collapsed into that case; update the
main() handling around fetchTLSConfig so it: call fetchTLSConfig(restConfig),
then if err == nil proceed as before, else if errors.Is(err, <the sentinel error
exported by fetchTLSConfig/openshifttls> /* e.g.
ErrTLSProfileMissing/ErrPolicyUnavailable */ then log and fall back to
Intermediate profile as currently done, otherwise treat it as a real read
failure (log error and return/exit or requeue) so you do NOT set
isOpenShiftCluster=false or skip SecurityProfileWatcher; use errors.Is to detect
the sentinel error and reference fetchTLSConfig, tlsAdherencePolicy,
honorClusterTLS and SecurityProfileWatcher when making the change.

In `@ptp-tools/Dockerfile.debug`:
- Around line 1-2: Replace running the debug image as root and installing
tcpdump with a non-root tcpdump user and grant tcpdump only the required
capabilities: after installing tcpdump in the Dockerfile (the RUN that installs
tcpdump), add creation of a tcpdump user (useradd tcpdump) and run setcap to
grant CAP_NET_RAW and CAP_NET_ADMIN on the tcpdump binary (setcap
cap_net_raw,cap_net_admin=ep /usr/sbin/tcpdump), then switch to that user with
USER tcpdump so the container no longer runs as root but tcpdump retains
necessary packet-capture privileges.

In `@scripts/run-tests.sh`:
- Around line 40-43: The script references DEBUG_IMAGE during option parsing but
never initializes it, causing a failure under set -u; initialize DEBUG_IMAGE
(e.g., set to an empty string or sensible default) before option parsing starts
so references like ${DEBUG_IMAGE} later (used in the run/tests logic) are safe;
update the top-level variable declarations or the option-parsing default block
(symbols: DEBUG_IMAGE and the --debug-image case in the option parsing) to
ensure DEBUG_IMAGE is always defined even if --debug-image is not provided.

In `@test/conformance/serial/ptp.go`:
- Around line 1117-1119: The test currently brings BC slave interfaces down
unconditionally via portEngine.TurnOffAndWaitFaulty(nic1.SlaveIf, nodeName),
which ignores the SKIP_INTERFACES safeguard; update the spec to first check the
SKIP_INTERFACES set (or call the existing helper used by the outage path) and
skip the test when nic1.SlaveIf (and the counterpart in the other spec range) is
present in SKIP_INTERFACES, logging a clear skip message and returning early
instead of calling TurnOffAndWaitFaulty; reference the SKIP_INTERFACES variable
and the TurnOffAndWaitFaulty(portEngine.*) usage to locate where to insert the
guard.
- Around line 3574-3605: When event.Enable() is true, do not silently disable
event validation on setup errors; update setupBCClockClassEvents so that
failures from event.CreateConsumerApp or event.MonitorPodLogsRegex cause the
test to fail (or explicitly skip with a clear message) instead of returning a
ctx that makes later verifyClockClassViaEvent a no-op. Locate
setupBCClockClassEvents and replace the current early return/log-only behavior
around event.CreateConsumerApp and the MonitorPodLogsRegex error branch with a
call that fails the spec (e.g., Ginkgo.Fail/Ginkgo.Skip or the project's
test-fail helper) including the error details, while keeping the existing
cleanup logic (eventCleanup, StopMonitor, event.PubSub.Close,
DeleteConsumerNamespace) intact.
- Around line 2054-2096: The FAULTY/SLAVE assertions currently use the full
slaveIf slice even though TurnAllPortsDown(skipInterfaces) leaves skipped NICs
up; update the test to build a filtered slice (e.g., filteredSlaveIf) by
excluding interfaces present in skipInterfaces (the map built from
SKIP_INTERFACES) and then create faultyRoles and slaveRoles using
len(filteredSlaveIf); call metrics.CheckClockRole with filteredSlaveIf (and
&slavePodNodeName) instead of slaveIf for both the FAULTY and SLAVE Eventually
checks so the assertions only cover the interfaces actually toggled by
TurnAllPortsDown/TurnAllPortsUp.

---

Nitpick comments:
In `@api/v1/ptpconfig_webhook.go`:
- Around line 59-61: Replace the deprecated WithCustomValidator usage and update
ptpConfigValidator to the typed validator API: change the call
ctrl.NewWebhookManagedBy(mgr,
r).WithCustomValidator(&ptpConfigValidator{}).Complete() to use
WithValidator(&ptpConfigValidator{}), and modify ptpConfigValidator's methods to
the typed signatures ValidateCreate(ctx context.Context, obj *PtpConfig)
(admission.Warnings, error), ValidateUpdate(ctx context.Context, oldObj, newObj
*PtpConfig) (admission.Warnings, error) and ValidateDelete(ctx context.Context,
obj *PtpConfig) (admission.Warnings, error), removing runtime.Object casts and
returning the same behavior with the new typed parameters.

In `@controllers/tls_profile_test.go`:
- Around line 128-174: The test TestTLSProfileTemplateRendering_LegacyAdherence
duplicates the cipher list; replace the hardcoded cipher string in the test
(both where data.Data["TLSCipherSuites"] is set and the assert.Contains expected
value) with a reference to the controller constant legacyCipherSuites so the
test uses the single source of truth; update imports if needed to access
legacyCipherSuites from the controller package and ensure the test assembles
rbacProxyArgs comparison against that constant.

In `@controllers/tls_watcher_test.go`:
- Around line 40-50: Add an explicit assertion in
TestSetTLSTemplateData_ModernProfile to verify the TLSCipherSuites value
produced by setTLSTemplateData: locate the test function
TestSetTLSTemplateData_ModernProfile, call r.setTLSTemplateData(&data) as
already done, then assert the expected value for data.Data["TLSCipherSuites"]
(e.g., assert.Equal(t, "", data.Data["TLSCipherSuites"])) to document that TLS
1.3 cipher suites are not configurable in Go.

In `@ptp-tools/Dockerfile.ptpop`:
- Line 2: Update the RUN instruction that installs binutils-gold to avoid
pulling recommended packages: change the apt-get install invocation in the
Dockerfile.ptpop RUN line to include --no-install-recommends (i.e., apt-get
update && apt-get install -y --no-install-recommends binutils-gold && rm -rf
/var/lib/apt/lists/*) so the image build is smaller and consistent with the rest
of the codebase.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 49423af0-2b12-4fdc-83c1-6a27c8e649f7

📥 Commits

Reviewing files that changed from the base of the PR and between 3c0739e and ee2ca6e.

⛔ Files ignored due to path filters (268)
  • go.sum is excluded by !**/*.sum
  • vendor/github.com/gogo/protobuf/AUTHORS is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/gogo/protobuf/CONTRIBUTORS is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/gogo/protobuf/LICENSE is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/gogo/protobuf/proto/Makefile is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/gogo/protobuf/proto/clone.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/gogo/protobuf/proto/custom_gogo.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/gogo/protobuf/proto/decode.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/gogo/protobuf/proto/deprecated.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/gogo/protobuf/proto/discard.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/gogo/protobuf/proto/duration.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/gogo/protobuf/proto/duration_gogo.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/gogo/protobuf/proto/encode.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/gogo/protobuf/proto/encode_gogo.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/gogo/protobuf/proto/equal.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/gogo/protobuf/proto/extensions.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/gogo/protobuf/proto/extensions_gogo.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/gogo/protobuf/proto/lib.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/gogo/protobuf/proto/lib_gogo.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/gogo/protobuf/proto/message_set.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/gogo/protobuf/proto/pointer_reflect.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/gogo/protobuf/proto/pointer_reflect_gogo.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/gogo/protobuf/proto/pointer_unsafe.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/gogo/protobuf/proto/pointer_unsafe_gogo.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/gogo/protobuf/proto/properties.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/gogo/protobuf/proto/properties_gogo.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/gogo/protobuf/proto/skip_gogo.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/gogo/protobuf/proto/table_marshal.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/gogo/protobuf/proto/table_marshal_gogo.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/gogo/protobuf/proto/table_merge.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/gogo/protobuf/proto/table_unmarshal.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/gogo/protobuf/proto/table_unmarshal_gogo.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/gogo/protobuf/proto/text.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/gogo/protobuf/proto/text_gogo.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/gogo/protobuf/proto/text_parser.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/gogo/protobuf/proto/timestamp.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/gogo/protobuf/proto/timestamp_gogo.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/gogo/protobuf/proto/wrappers.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/gogo/protobuf/proto/wrappers_gogo.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/gogo/protobuf/sortkeys/sortkeys.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/openshift/api/config/v1/types_apiserver.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/openshift/api/config/v1/types_authentication.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/openshift/api/config/v1/types_cluster_version.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/openshift/api/config/v1/types_infrastructure.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/openshift/api/config/v1/types_ingress.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/openshift/api/config/v1/types_insights.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/openshift/api/config/v1/types_network.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/openshift/api/config/v1/types_tlssecurityprofile.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/openshift/api/config/v1/zz_generated.deepcopy.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/openshift/api/config/v1/zz_generated.featuregated-crd-manifests.yaml is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/openshift/api/config/v1/zz_generated.swagger_doc_generated.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/openshift/api/config/v1alpha1/register.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/openshift/api/config/v1alpha1/types_backup.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/openshift/api/config/v1alpha1/types_cluster_image_policy.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/openshift/api/config/v1alpha1/types_cluster_monitoring.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/openshift/api/config/v1alpha1/types_crio_credential_provider_config.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/openshift/api/config/v1alpha1/types_image_policy.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/openshift/api/config/v1alpha1/types_insights.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/openshift/api/config/v1alpha1/types_pki.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/openshift/api/config/v1alpha1/zz_generated.deepcopy.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/openshift/api/config/v1alpha1/zz_generated.featuregated-crd-manifests.yaml is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/openshift/api/config/v1alpha1/zz_generated.swagger_doc_generated.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/openshift/api/config/v1alpha2/types_insights.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/openshift/api/config/v1alpha2/zz_generated.featuregated-crd-manifests.yaml is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/openshift/controller-runtime-common/pkg/tls/controller.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/openshift/controller-runtime-common/pkg/tls/tls.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/openshift/library-go/pkg/crypto/tls_adherence.go is excluded by !vendor/**, !**/vendor/**
  • vendor/google.golang.org/protobuf/internal/encoding/tag/tag.go is excluded by !vendor/**, !**/vendor/**
  • vendor/google.golang.org/protobuf/internal/encoding/text/decode.go is excluded by !vendor/**, !**/vendor/**
  • vendor/google.golang.org/protobuf/internal/filedesc/desc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/google.golang.org/protobuf/internal/filedesc/desc_init.go is excluded by !vendor/**, !**/vendor/**
  • vendor/google.golang.org/protobuf/internal/filedesc/desc_lazy.go is excluded by !vendor/**, !**/vendor/**
  • vendor/google.golang.org/protobuf/internal/filedesc/editions.go is excluded by !vendor/**, !**/vendor/**
  • vendor/google.golang.org/protobuf/internal/genid/descriptor_gen.go is excluded by !vendor/**, !**/vendor/**
  • vendor/google.golang.org/protobuf/internal/impl/codec_map.go is excluded by !vendor/**, !**/vendor/**
  • vendor/google.golang.org/protobuf/internal/impl/decode.go is excluded by !vendor/**, !**/vendor/**
  • vendor/google.golang.org/protobuf/internal/impl/validate.go is excluded by !vendor/**, !**/vendor/**
  • vendor/google.golang.org/protobuf/internal/version/version.go is excluded by !vendor/**, !**/vendor/**
  • vendor/google.golang.org/protobuf/proto/decode.go is excluded by !vendor/**, !**/vendor/**
  • vendor/google.golang.org/protobuf/types/descriptorpb/descriptor.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/google.golang.org/protobuf/types/known/timestamppb/timestamp.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/admission/v1/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/admission/v1/generated.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/admission/v1/generated.proto is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/admission/v1/generated.protomessage.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/admission/v1/types.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/admission/v1/types_swagger_doc_generated.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/admission/v1/zz_generated.model_name.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/admission/v1beta1/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/admission/v1beta1/generated.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/admission/v1beta1/generated.proto is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/admission/v1beta1/generated.protomessage.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/admission/v1beta1/types.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/admission/v1beta1/types_swagger_doc_generated.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/admission/v1beta1/zz_generated.model_name.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/admissionregistration/v1/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/admissionregistration/v1/generated.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/admissionregistration/v1/generated.protomessage.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/admissionregistration/v1/types.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/admissionregistration/v1/zz_generated.model_name.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/admissionregistration/v1alpha1/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/admissionregistration/v1alpha1/generated.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/admissionregistration/v1alpha1/generated.protomessage.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/admissionregistration/v1alpha1/types.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/admissionregistration/v1alpha1/zz_generated.model_name.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/admissionregistration/v1beta1/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/admissionregistration/v1beta1/generated.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/admissionregistration/v1beta1/generated.proto is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/admissionregistration/v1beta1/generated.protomessage.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/admissionregistration/v1beta1/types.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/admissionregistration/v1beta1/zz_generated.model_name.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/apidiscovery/v2/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/apidiscovery/v2/generated.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/apidiscovery/v2/generated.protomessage.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/apidiscovery/v2/zz_generated.model_name.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/apidiscovery/v2beta1/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/apidiscovery/v2beta1/generated.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/apidiscovery/v2beta1/generated.protomessage.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/apidiscovery/v2beta1/zz_generated.model_name.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/apiserverinternal/v1alpha1/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/apiserverinternal/v1alpha1/generated.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/apiserverinternal/v1alpha1/generated.protomessage.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/apiserverinternal/v1alpha1/zz_generated.model_name.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/apps/v1/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/apps/v1/generated.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/apps/v1/generated.proto is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/apps/v1/generated.protomessage.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/apps/v1/types.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/apps/v1/types_swagger_doc_generated.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/apps/v1/zz_generated.model_name.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/apps/v1beta1/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/apps/v1beta1/generated.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/apps/v1beta1/generated.proto is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/apps/v1beta1/generated.protomessage.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/apps/v1beta1/types.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/apps/v1beta1/types_swagger_doc_generated.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/apps/v1beta1/zz_generated.model_name.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/apps/v1beta2/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/apps/v1beta2/generated.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/apps/v1beta2/generated.proto is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/apps/v1beta2/generated.protomessage.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/apps/v1beta2/types.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/apps/v1beta2/types_swagger_doc_generated.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/apps/v1beta2/zz_generated.model_name.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/authentication/v1/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/authentication/v1/generated.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/authentication/v1/generated.protomessage.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/authentication/v1/zz_generated.model_name.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/authentication/v1alpha1/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/authentication/v1alpha1/generated.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/authentication/v1alpha1/generated.protomessage.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/authentication/v1alpha1/zz_generated.model_name.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/authentication/v1beta1/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/authentication/v1beta1/generated.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/authentication/v1beta1/generated.protomessage.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/authentication/v1beta1/zz_generated.model_name.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/authorization/v1/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/authorization/v1/generated.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/authorization/v1/generated.protomessage.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/authorization/v1/zz_generated.model_name.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/authorization/v1beta1/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/authorization/v1beta1/generated.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/authorization/v1beta1/generated.protomessage.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/authorization/v1beta1/zz_generated.model_name.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/autoscaling/v1/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/autoscaling/v1/generated.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/autoscaling/v1/generated.protomessage.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/autoscaling/v1/zz_generated.model_name.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/autoscaling/v2/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/autoscaling/v2/generated.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/autoscaling/v2/generated.proto is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/autoscaling/v2/generated.protomessage.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/autoscaling/v2/types.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/autoscaling/v2/types_swagger_doc_generated.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/autoscaling/v2/zz_generated.model_name.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/autoscaling/v2beta1/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/autoscaling/v2beta1/generated.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/autoscaling/v2beta1/generated.protomessage.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/autoscaling/v2beta1/zz_generated.model_name.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/autoscaling/v2beta2/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/autoscaling/v2beta2/generated.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/autoscaling/v2beta2/generated.protomessage.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/autoscaling/v2beta2/zz_generated.model_name.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/batch/v1/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/batch/v1/generated.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/batch/v1/generated.proto is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/batch/v1/generated.protomessage.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/batch/v1/types.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/batch/v1/types_swagger_doc_generated.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/batch/v1/zz_generated.model_name.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/batch/v1beta1/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/batch/v1beta1/generated.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/batch/v1beta1/generated.protomessage.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/batch/v1beta1/zz_generated.model_name.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/certificates/v1/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/certificates/v1/generated.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/certificates/v1/generated.proto is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/certificates/v1/generated.protomessage.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/certificates/v1/types.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/certificates/v1/zz_generated.model_name.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/certificates/v1alpha1/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/certificates/v1alpha1/generated.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/certificates/v1alpha1/generated.proto is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/certificates/v1alpha1/generated.protomessage.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/certificates/v1alpha1/register.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/certificates/v1alpha1/types.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/certificates/v1alpha1/types_swagger_doc_generated.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/certificates/v1alpha1/zz_generated.deepcopy.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/certificates/v1alpha1/zz_generated.model_name.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/certificates/v1alpha1/zz_generated.prerelease-lifecycle.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/certificates/v1beta1/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/certificates/v1beta1/generated.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/certificates/v1beta1/generated.proto is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/certificates/v1beta1/generated.protomessage.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/certificates/v1beta1/register.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/certificates/v1beta1/types.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/certificates/v1beta1/types_swagger_doc_generated.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/certificates/v1beta1/zz_generated.deepcopy.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/certificates/v1beta1/zz_generated.model_name.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/certificates/v1beta1/zz_generated.prerelease-lifecycle.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/coordination/v1/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/coordination/v1/generated.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/coordination/v1/generated.protomessage.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/coordination/v1/zz_generated.model_name.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/coordination/v1alpha2/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/coordination/v1alpha2/generated.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/coordination/v1alpha2/generated.protomessage.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/coordination/v1alpha2/zz_generated.model_name.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/coordination/v1beta1/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/coordination/v1beta1/generated.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/coordination/v1beta1/generated.protomessage.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/coordination/v1beta1/zz_generated.model_name.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/core/v1/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/core/v1/generated.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/core/v1/generated.proto is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/core/v1/generated.protomessage.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/core/v1/toleration.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/core/v1/types.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/core/v1/types_swagger_doc_generated.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/core/v1/zz_generated.deepcopy.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/core/v1/zz_generated.model_name.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/discovery/v1/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/discovery/v1/generated.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/discovery/v1/generated.proto is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/discovery/v1/generated.protomessage.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/discovery/v1/types.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/discovery/v1/types_swagger_doc_generated.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/discovery/v1/zz_generated.model_name.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/discovery/v1beta1/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/discovery/v1beta1/generated.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/discovery/v1beta1/generated.proto is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/discovery/v1beta1/generated.protomessage.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/discovery/v1beta1/types.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/discovery/v1beta1/types_swagger_doc_generated.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/discovery/v1beta1/zz_generated.model_name.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/events/v1/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/events/v1/generated.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/events/v1/generated.protomessage.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/events/v1/zz_generated.model_name.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/events/v1beta1/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/events/v1beta1/generated.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/events/v1beta1/generated.protomessage.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/events/v1beta1/zz_generated.model_name.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/extensions/v1beta1/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/extensions/v1beta1/generated.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/extensions/v1beta1/generated.proto is excluded by !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/extensions/v1beta1/generated.protomessage.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/k8s.io/api/extensions/v1beta1/types.go is excluded by !vendor/**, !**/vendor/**
📒 Files selected for processing (32)
  • Dockerfile
  • OWNERS
  • api/v1/nodeptpdevice_types.go
  • api/v1/ptpconfig_webhook.go
  • api/v1/ptpoperatorconfig_webhook.go
  • api/v1/zz_generated.deepcopy.go
  • bindata/linuxptp/ptp-daemon.yaml
  • bundle/manifests/ptp-operator.clusterserviceversion.yaml
  • bundle/manifests/ptp.openshift.io_nodeptpdevices.yaml
  • config/crd/bases/ptp.openshift.io_nodeptpdevices.yaml
  • config/manifests/bases/ptp-operator.clusterserviceversion.yaml
  • config/prometheus/monitor.yaml
  • controllers/ptpoperatorconfig_controller.go
  • controllers/tls_profile_test.go
  • controllers/tls_watcher_test.go
  • go.mod
  • main.go
  • manifests/stable/ptp-operator.clusterserviceversion.yaml
  • manifests/stable/ptp.openshift.io_nodeptpdevices.yaml
  • must-gather/collection-scripts/gather
  • ptp-tools/Dockerfile.cep
  • ptp-tools/Dockerfile.debug
  • ptp-tools/Dockerfile.krp
  • ptp-tools/Dockerfile.lptpd
  • ptp-tools/Dockerfile.ptpop
  • ptp-tools/Makefile
  • scripts/k8s-start.sh
  • scripts/run-on-vm.sh
  • scripts/run-tests.sh
  • test/conformance/serial/ptp.go
  • test/pkg/ptptesthelper/ptptesthelper.go
  • test/pkg/testconfig/testconfig.go

Comment on lines +1 to +3
FROM registry.access.redhat.com/ubi9/go-toolset:1.25 AS builder
# Set as root to permission issues as its only for the builder
USER root
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Set a non-root user for the runtime stage.

Line 3 explicitly sets root in build stage, and the runtime stage still has no USER, so the shipped container runs as root by default.

Proposed change (runtime stage)
 FROM quay.io/centos/centos:stream9
 COPY --from=builder /go/src/github.com/k8snetworkplumbingwg/ptp-operator/build/_output/bin/ptp-operator /usr/local/bin/
 COPY --from=builder /go/src/github.com/k8snetworkplumbingwg/ptp-operator/manifests /manifests
 COPY bindata /bindata
@@
+USER 65532
 ENTRYPOINT ["/usr/local/bin/ptp-operator"]

As per coding guidelines, "Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."

🧰 Tools
🪛 Trivy (0.69.3)

[error] 3-3: Image user should not be 'root'

Last USER command in Dockerfile should not be 'root'

Rule: DS-0002

Learn more

(IaC/Dockerfile)


[error] 1-1: Image user should not be 'root'

Specify at least 1 USER command in Dockerfile with non-root user as argument

Rule: DS-0002

Learn more

(IaC/Dockerfile)


[error] 1-1: Image user should not be 'root'

Specify at least 1 USER command in Dockerfile with non-root user as argument

Rule: DS-0002

Learn more

(IaC/Dockerfile)


[error] 1-1: Image user should not be 'root'

Specify at least 1 USER command in Dockerfile with non-root user as argument

Rule: DS-0002

Learn more

(IaC/Dockerfile)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Dockerfile` around lines 1 - 3, The Dockerfile currently sets USER root in
the builder stage (FROM registry.access.redhat.com/ubi9/go-toolset:1.25 AS
builder / USER root) and never switches to a non-root user for the final runtime
image; update the Dockerfile to create or use a non-root user/group in the build
(or final) stage and add a USER <nonroot> directive in the runtime stage so the
shipped container does not run as root; ensure any runtime-owned files or
directories are chowned to that user (e.g., during the builder stage or in a
dedicated setup step) and reference the created username in the final USER
instruction.

Comment on lines +90 to 117
// Fetch the TLS security profile and adherence policy from the APIServer CR.
// Defaults to the Intermediate profile if the CR is not accessible.
tlsProfileSpec, err := fetchTLSProfile(restConfig)
tlsProfileSpec, tlsAdherencePolicy, err := fetchTLSConfig(restConfig)
isOpenShiftCluster := err == nil
if !isOpenShiftCluster {
setupLog.Info("unable to fetch TLS profile from APIServer CR, using Intermediate profile", "error", err)
setupLog.Info("unable to fetch TLS config from APIServer CR, using Intermediate profile", "error", err)
tlsProfileSpec, _ = openshifttls.GetTLSProfileSpec(nil)
}
setupLog.Info("TLS security profile resolved", "minTLSVersion", tlsProfileSpec.MinTLSVersion, "ciphers", tlsProfileSpec.Ciphers)
tlsOption, unsupportedCiphers := openshifttls.NewTLSConfigFromProfile(tlsProfileSpec)
if len(unsupportedCiphers) > 0 {
setupLog.Info("some ciphers from the TLS profile are not supported", "unsupportedCiphers", unsupportedCiphers)
honorClusterTLS := libgocrypto.ShouldHonorClusterTLSProfile(tlsAdherencePolicy)
setupLog.Info("TLS security profile resolved",
"minTLSVersion", tlsProfileSpec.MinTLSVersion,
"ciphers", tlsProfileSpec.Ciphers,
"tlsAdherence", tlsAdherencePolicy,
"honorClusterTLSProfile", honorClusterTLS)

var tlsOption func(*tls.Config)
var tlsProfileSpecPtr *configv1.TLSProfileSpec
if honorClusterTLS {
tlsProfileSpecPtr = &tlsProfileSpec
var unsupportedCiphers []string
tlsOption, unsupportedCiphers = openshifttls.NewTLSConfigFromProfile(tlsProfileSpec)
if len(unsupportedCiphers) > 0 {
setupLog.Info("some ciphers from the TLS profile are not supported", "unsupportedCiphers", unsupportedCiphers)
}
} else {
setupLog.Info("TLS adherence is legacy, using default TLS configuration")
tlsOption = func(*tls.Config) {} // no-op: use Go defaults
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Don't collapse all APIServer TLS read errors into the non-OpenShift fallback.

fetchTLSConfig() now distinguishes the supported missing-policy case, but main() still treats any returned error here as "use fallback TLS". A transient or RBAC failure while reading the APIServer config will silently change the TLS posture and skip SecurityProfileWatcher startup on an actual OpenShift cluster. Please separate the expected "cluster TLS config unavailable" case from real read failures.

Also applies to: 334-361

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@main.go` around lines 90 - 117, fetchTLSConfig() can return a sentinel
"missing-policy" result that should be treated as the non-OpenShift fallback,
but other errors (transient/RBAC/read failures) must not be collapsed into that
case; update the main() handling around fetchTLSConfig so it: call
fetchTLSConfig(restConfig), then if err == nil proceed as before, else if
errors.Is(err, <the sentinel error exported by fetchTLSConfig/openshifttls> /*
e.g. ErrTLSProfileMissing/ErrPolicyUnavailable */ then log and fall back to
Intermediate profile as currently done, otherwise treat it as a real read
failure (log error and return/exit or requeue) so you do NOT set
isOpenShiftCluster=false or skip SecurityProfileWatcher; use errors.Is to detect
the sentinel error and reference fetchTLSConfig, tlsAdherencePolicy,
honorClusterTLS and SecurityProfileWatcher when making the change.

Comment on lines +1 to +2
FROM quay.io/centos/centos:stream9
RUN yum install -y tcpdump && yum clean all
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail
# Verify where debug image is consumed
rg -n -- '--debug-image|test:debug|Dockerfile.debug'

# Check whether manifests/jobs set explicit security context and capabilities
rg -n --iglob '*.{yaml,yml,json}' 'securityContext|runAsUser|capabilities|privileged'

Repository: openshift/ptp-operator

Length of output: 1418


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Check how debug image is used in run-on-vm.sh
echo "=== run-on-vm.sh usage of debug image ==="
sed -n '85,95p' scripts/run-on-vm.sh

echo -e "\n=== run-tests.sh usage of debug image ==="
sed -n '40,50p' scripts/run-tests.sh

echo -e "\n=== gather script usage ==="
sed -n '20,30p' must-gather/collection-scripts/gather

echo -e "\n=== Check Dockerfile.debug for any additional context ==="
cat ptp-tools/Dockerfile.debug

echo -e "\n=== Check if ptp-tools/Makefile uses Dockerfile.debug ==="
if [ -f ptp-tools/Makefile ]; then
  grep -n -i "dockerfile.debug\|debug.*image" ptp-tools/Makefile || echo "No matches found"
else
  echo "ptp-tools/Makefile not found"
fi

echo -e "\n=== Check root Makefile for debug image build ==="
grep -n -i "dockerfile.debug\|debug.*image" Makefile 2>/dev/null | head -20 || echo "No matches"

Repository: openshift/ptp-operator

Length of output: 1119


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Full gather script to see DEBUG_IMAGE usage ==="
cat must-gather/collection-scripts/gather | head -100

echo -e "\n=== Search for how DEBUG_IMAGE is used later in gather ==="
grep -n "DEBUG_IMAGE" must-gather/collection-scripts/gather

echo -e "\n=== Check if there's a pod/container run command with DEBUG_IMAGE ==="
rg -n -A5 'DEBUG_IMAGE' must-gather/collection-scripts/

echo -e "\n=== Check tcpdump requirements for capabilities ==="
# Search for any security context or capability specs that might apply to debug
rg -n 'tcpdump|CAP_' ptp-tools/ 2>/dev/null || echo "No tcpdump/CAP refs in ptp-tools"

echo -e "\n=== Check if gather script creates pods/containers ==="
rg -n 'kubectl.*run|oc.*run|podman.*run' must-gather/ || echo "No container run commands found"

Repository: openshift/ptp-operator

Length of output: 4841


Use Linux capabilities instead of root for tcpdump in debug image.

The debug image defaults to root unnecessarily. Since tcpdump only needs network packet capture privileges, use Linux capabilities (CAP_NET_RAW and CAP_NET_ADMIN) instead. This reduces the blast radius if the image is accidentally used outside debug contexts.

Example:

FROM quay.io/centos/centos:stream9
RUN yum install -y tcpdump && yum clean all
RUN useradd -m tcpdump && setcap cap_net_raw,cap_net_admin=ep /usr/sbin/tcpdump
USER tcpdump
🧰 Tools
🪛 Trivy (0.69.3)

[error] 1-1: Image user should not be 'root'

Specify at least 1 USER command in Dockerfile with non-root user as argument

Rule: DS-0002

Learn more

(IaC/Dockerfile)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@ptp-tools/Dockerfile.debug` around lines 1 - 2, Replace running the debug
image as root and installing tcpdump with a non-root tcpdump user and grant
tcpdump only the required capabilities: after installing tcpdump in the
Dockerfile (the RUN that installs tcpdump), add creation of a tcpdump user
(useradd tcpdump) and run setcap to grant CAP_NET_RAW and CAP_NET_ADMIN on the
tcpdump binary (setcap cap_net_raw,cap_net_admin=ep /usr/sbin/tcpdump), then
switch to that user with USER tcpdump so the container no longer runs as root
but tcpdump retains necessary packet-capture privileges.

Comment on lines +40 to +43
--must-gather-image)
MUST_GATHER_IMAGE="$2"; shift 2 ;;
--debug-image)
DEBUG_IMAGE="$2"; shift 2 ;;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Missing DEBUG_IMAGE initialization will cause runtime error with set -u.

DEBUG_IMAGE is parsed on line 42-43 but never initialized with a default value. Since the script uses set -u (line 17), referencing ${DEBUG_IMAGE} on line 127 will fail if --debug-image is not provided.

🐛 Proposed fix
 LINUXPTP_DAEMON_IMAGE=""
 MUST_GATHER_IMAGE=""
+DEBUG_IMAGE=""
 
 while [[ $# -gt 0 ]]; do
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@scripts/run-tests.sh` around lines 40 - 43, The script references DEBUG_IMAGE
during option parsing but never initializes it, causing a failure under set -u;
initialize DEBUG_IMAGE (e.g., set to an empty string or sensible default) before
option parsing starts so references like ${DEBUG_IMAGE} later (used in the
run/tests logic) are safe; update the top-level variable declarations or the
option-parsing default block (symbols: DEBUG_IMAGE and the --debug-image case in
the option parsing) to ensure DEBUG_IMAGE is always defined even if
--debug-image is not provided.

Comment on lines +1117 to +1119
// Step 2: cut upstream sync by bringing slave interface down
By(fmt.Sprintf("Step 2: Locking NIC-1 PTP source (bringing down %s)", nic1.SlaveIf))
portEngine.TurnOffAndWaitFaulty(nic1.SlaveIf, nodeName)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Honor SKIP_INTERFACES before dropping BC slave links.

These specs bring discovered BC slave interfaces down unconditionally. The outage path later in this file already treats that operation as risky enough to require a skip list, and on clusters where one of these NICs also carries node traffic this can strand the node and cascade failures. Reuse the same safeguard here, or skip the spec when the target interface is protected.

Also applies to: 1176-1189

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/conformance/serial/ptp.go` around lines 1117 - 1119, The test currently
brings BC slave interfaces down unconditionally via
portEngine.TurnOffAndWaitFaulty(nic1.SlaveIf, nodeName), which ignores the
SKIP_INTERFACES safeguard; update the spec to first check the SKIP_INTERFACES
set (or call the existing helper used by the outage path) and skip the test when
nic1.SlaveIf (and the counterpart in the other spec range) is present in
SKIP_INTERFACES, logging a clear skip message and returning early instead of
calling TurnOffAndWaitFaulty; reference the SKIP_INTERFACES variable and the
TurnOffAndWaitFaulty(portEngine.*) usage to locate where to insert the guard.

Comment on lines +2054 to +2096
By("Setting all slave interfaces down")
skippedInterfacesStr, isSet := os.LookupEnv("SKIP_INTERFACES")
if !isSet {
Skip("Mandatory to provide skipped interface to avoid making a node disconnected from the cluster")
}
skipInterfaces := make(map[string]bool)
for _, val := range strings.Split(skippedInterfacesStr, ",") {
skipInterfaces[val] = true
}
err := portEngine.TurnAllPortsDown(skipInterfaces)
Expect(err).To(BeNil())
DeferCleanup(func() {
portEngine.TurnAllPortsUp()
})

faultyRoles := make([]metrics.MetricRole, len(slaveIf))
for i := range faultyRoles {
faultyRoles[i] = metrics.MetricRoleFaulty
}
slaveRoles := make([]metrics.MetricRole, len(slaveIf))
for i := range slaveRoles {
slaveRoles[i] = metrics.MetricRoleSlave
}

By("Checking that all slave port roles are FAULTY after wait")
Eventually(func() error {
return metrics.CheckClockRole(faultyRoles, slaveIf, &slavePodNodeName)
}, 5*time.Minute, 10*time.Second).Should(BeNil())

By("Checking clock class has degraded away from Locked (6)")
Eventually(func() bool {
return !checkClockClassStateReturnBool(fullConfig, strconv.Itoa(int(fbprotocol.ClockClass6)))
}, 5*time.Minute, 10*time.Second).Should(BeTrue(),
"expected clock class to degrade from Locked (6) after upstream link loss")

By("Setting all slave interfaces up")
err = portEngine.TurnAllPortsUp()
Expect(err).To(BeNil())

By("Checking that all slave port roles are SLAVE after wait")
Eventually(func() error {
return metrics.CheckClockRole(slaveRoles, slaveIf, &slavePodNodeName)
}, 5*time.Minute, 10*time.Second).Should(BeNil())
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Use the filtered interface set for the FAULTY/SLAVE assertions.

TurnAllPortsDown(skipInterfaces) intentionally leaves skipped NICs up, but faultyRoles and slaveRoles are still built against the full slaveIf slice. If SKIP_INTERFACES overlaps any BC slave port, these checks can never pass.

Suggested fix
 				skipInterfaces := make(map[string]bool)
 				for _, val := range strings.Split(skippedInterfacesStr, ",") {
 					skipInterfaces[val] = true
 				}
+				testIfs := make([]string, 0, len(slaveIf))
+				for _, iface := range slaveIf {
+					if !skipInterfaces[iface] {
+						testIfs = append(testIfs, iface)
+					}
+				}
+				Expect(testIfs).ToNot(BeEmpty(), "all BC slave interfaces were skipped")
 				err := portEngine.TurnAllPortsDown(skipInterfaces)
 				Expect(err).To(BeNil())
 				DeferCleanup(func() {
 					portEngine.TurnAllPortsUp()
 				})
 
-				faultyRoles := make([]metrics.MetricRole, len(slaveIf))
+				faultyRoles := make([]metrics.MetricRole, len(testIfs))
 				for i := range faultyRoles {
 					faultyRoles[i] = metrics.MetricRoleFaulty
 				}
-				slaveRoles := make([]metrics.MetricRole, len(slaveIf))
+				slaveRoles := make([]metrics.MetricRole, len(testIfs))
 				for i := range slaveRoles {
 					slaveRoles[i] = metrics.MetricRoleSlave
 				}
 
 				By("Checking that all slave port roles are FAULTY after wait")
 				Eventually(func() error {
-					return metrics.CheckClockRole(faultyRoles, slaveIf, &slavePodNodeName)
+					return metrics.CheckClockRole(faultyRoles, testIfs, &slavePodNodeName)
 				}, 5*time.Minute, 10*time.Second).Should(BeNil())
@@
 				By("Checking that all slave port roles are SLAVE after wait")
 				Eventually(func() error {
-					return metrics.CheckClockRole(slaveRoles, slaveIf, &slavePodNodeName)
+					return metrics.CheckClockRole(slaveRoles, testIfs, &slavePodNodeName)
 				}, 5*time.Minute, 10*time.Second).Should(BeNil())
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/conformance/serial/ptp.go` around lines 2054 - 2096, The FAULTY/SLAVE
assertions currently use the full slaveIf slice even though
TurnAllPortsDown(skipInterfaces) leaves skipped NICs up; update the test to
build a filtered slice (e.g., filteredSlaveIf) by excluding interfaces present
in skipInterfaces (the map built from SKIP_INTERFACES) and then create
faultyRoles and slaveRoles using len(filteredSlaveIf); call
metrics.CheckClockRole with filteredSlaveIf (and &slavePodNodeName) instead of
slaveIf for both the FAULTY and SLAVE Eventually checks so the assertions only
cover the interfaces actually toggled by TurnAllPortsDown/TurnAllPortsUp.

Comment on lines +3574 to +3605
func setupBCClockClassEvents(nodeName string) bcEventContext {
ctx := bcEventContext{}
if !event.Enable() {
return ctx
}
logrus.Info("Deploy consumer app for BC clock class event monitoring")
if createErr := event.CreateConsumerApp(nodeName); createErr != nil {
logrus.Warnf("PTP events not available: %s; skipping event checks", createErr)
return ctx
}
time.Sleep(10 * time.Second)
event.InitPubSub()
var eventCleanup func()
ctx.subs, eventCleanup = event.SubscribeToGMChangeEvents(100, true, 60*time.Second)
termMonitor, monErr := event.MonitorPodLogsRegex()
if monErr != nil {
logrus.Warnf("Could not start event monitoring: %s; skipping event checks", monErr)
} else {
ctx.available = true
}
DeferCleanup(func() {
if termMonitor != nil {
stopMonitor(termMonitor)
}
eventCleanup()
event.PubSub.Close()
if deleteErr := event.DeleteConsumerNamespace(); deleteErr != nil {
logrus.Debugf("Deleting consumer namespace failed: %s", deleteErr)
}
})
return ctx
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Don't silently disable Event API validation after setup failures.

Once event.Enable() is true, failing consumer creation or log monitoring means the event path is broken. Returning available=false makes every later verifyClockClassViaEvent call a no-op, so these specs can pass without validating Event API behavior at all.

Suggested fix
 func setupBCClockClassEvents(nodeName string) bcEventContext {
 	ctx := bcEventContext{}
 	if !event.Enable() {
 		return ctx
 	}
 	logrus.Info("Deploy consumer app for BC clock class event monitoring")
-	if createErr := event.CreateConsumerApp(nodeName); createErr != nil {
-		logrus.Warnf("PTP events not available: %s; skipping event checks", createErr)
-		return ctx
-	}
+	Expect(event.CreateConsumerApp(nodeName)).To(Succeed(), "event consumer setup failed")
 	time.Sleep(10 * time.Second)
 	event.InitPubSub()
 	var eventCleanup func()
 	ctx.subs, eventCleanup = event.SubscribeToGMChangeEvents(100, true, 60*time.Second)
 	termMonitor, monErr := event.MonitorPodLogsRegex()
-	if monErr != nil {
-		logrus.Warnf("Could not start event monitoring: %s; skipping event checks", monErr)
-	} else {
-		ctx.available = true
-	}
+	Expect(monErr).NotTo(HaveOccurred(), "event monitoring failed")
+	ctx.available = true
 	DeferCleanup(func() {
 		if termMonitor != nil {
 			stopMonitor(termMonitor)
 		}
 		eventCleanup()
-		event.PubSub.Close()
+		if event.PubSub != nil {
+			event.PubSub.Close()
+		}
 		if deleteErr := event.DeleteConsumerNamespace(); deleteErr != nil {
 			logrus.Debugf("Deleting consumer namespace failed: %s", deleteErr)
 		}
 	})
 	return ctx
 }

Also applies to: 3609-3614

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/conformance/serial/ptp.go` around lines 3574 - 3605, When event.Enable()
is true, do not silently disable event validation on setup errors; update
setupBCClockClassEvents so that failures from event.CreateConsumerApp or
event.MonitorPodLogsRegex cause the test to fail (or explicitly skip with a
clear message) instead of returning a ctx that makes later
verifyClockClassViaEvent a no-op. Locate setupBCClockClassEvents and replace the
current early return/log-only behavior around event.CreateConsumerApp and the
MonitorPodLogsRegex error branch with a call that fails the spec (e.g.,
Ginkgo.Fail/Ginkgo.Skip or the project's test-fail helper) including the error
details, while keeping the existing cleanup logic (eventCleanup, StopMonitor,
event.PubSub.Close, DeleteConsumerNamespace) intact.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. jira/severity-important Referenced Jira bug's severity is important for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants