DPTP-5090: Add unsigned TLS posture attestation emitter with component rollup

[richardsonnick]

Emit a --attestation-file JSON document that rolls per-port scan results up by OpenShift component (PASS/FAIL/SKIP) for CI to Cosign-sign. Include ClusterVersion subject hints when scanning with --all-pods.

[openshift-ci-robot]

@richardsonnick: This pull request references DPTP-5090 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 task to target the "5.0.0" version, but no target version was set.

Details

In response to this:

Emit a --attestation-file JSON document that rolls per-port scan results up by OpenShift component (PASS/FAIL/SKIP) for CI to Cosign-sign. Include ClusterVersion subject hints when scanning with --all-pods.

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]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: richardsonnick

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:

• OWNERS [richardsonnick]

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

[qodo-for-rh-openshift]

Code Review by Qodo

🐞 Bugs (3) 📘 Rule violations (1) 📜 Skill insights (0)

Context used

✅ Compliance rules (platform): 10 rules

1. BuildTLSPostureAttestation tests not table-driven 📘 Rule violation ▣ Testability

Description

BuildTLSPostureAttestation is tested across multiple input/expected-output scenarios using
separate, largely similar test functions instead of a table-driven test loop. This reduces
maintainability and increases the chance of inconsistent assertions when adding new scenarios.

Code

internal/output/attestation_test.go[R15-177]

+func TestBuildTLSPostureAttestationPQCRollup(t *testing.T) {
+	t.Parallel()
+
+	results := scanner.ScanResults{
+		Timestamp: "2026-07-23T12:00:00Z",
+		IPResults: []scanner.IPResult{
+			{
+				IP: "10.0.0.1",
+				OpenshiftComponent: &k8s.OpenshiftComponent{
+					Component:           "kube-apiserver",
+					MaintainerComponent: "ocp",
+				},
+				Pod: &k8s.PodInfo{Name: "apiserver", Image: "quay.io/openshift/kube-apiserver@sha256:aaa"},
+				PortResults: []scanner.PortResult{
+					{
+						Port:           6443,
+						Status:         scanner.StatusOK,
+						TLS13Supported: true,
+						MLKEMSupported: true,
+					},
+					{
+						Port:   8080,
+						Status: scanner.StatusLocalhostOnly,
+					},
+				},
+			},
+			{
+				IP: "10.0.0.2",
+				OpenshiftComponent: &k8s.OpenshiftComponent{
+					Component: "openshift-ingress",
+				},
+				Pod: &k8s.PodInfo{Name: "router", Image: "quay.io/openshift/router@sha256:bbb"},
+				PortResults: []scanner.PortResult{
+					{
+						Port:           443,
+						Status:         scanner.StatusOK,
+						TLS13Supported: true,
+						MLKEMSupported: false,
+					},
+				},
+			},
+			{
+				IP: "10.0.0.3",
+				PortResults: []scanner.PortResult{
+					{Port: 9090, Status: scanner.StatusNoTLS},
+				},
+			},
+		},
+	}
+
+	doc := BuildTLSPostureAttestation(results, true, AttestationMeta{
+		ScannerVersion: "test",
+		ScannerCommit:  "abc",
+		ClusterVersion: &k8s.ClusterVersionInfo{Version: "4.22.0", Image: "quay.io/ocp@sha256:ccc"},
+	})
+
+	if doc.PredicateType != TLSPosturePredicateType {
+		t.Errorf("PredicateType = %q", doc.PredicateType)
+	}
+	if doc.Predicate.PolicyBar != PolicyBarPQC {
+		t.Errorf("PolicyBar = %q, want %q", doc.Predicate.PolicyBar, PolicyBarPQC)
+	}
+	if doc.Predicate.Result != OverallResultFail {
+		t.Errorf("Result = %q, want FAIL", doc.Predicate.Result)
+	}
+	if doc.Predicate.ClusterVersion == nil || doc.Predicate.ClusterVersion.Version != "4.22.0" {
+		t.Errorf("ClusterVersion = %+v", doc.Predicate.ClusterVersion)
+	}
+	if doc.Predicate.Summary.ComponentsTotal != 3 {
+		t.Errorf("ComponentsTotal = %d, want 3", doc.Predicate.Summary.ComponentsTotal)
+	}
+	if doc.Predicate.Summary.ComponentsPassed != 1 || doc.Predicate.Summary.ComponentsFailed != 1 || doc.Predicate.Summary.ComponentsSkipped != 1 {
+		t.Errorf("summary pass/fail/skip = %d/%d/%d, want 1/1/1",
+			doc.Predicate.Summary.ComponentsPassed, doc.Predicate.Summary.ComponentsFailed, doc.Predicate.Summary.ComponentsSkipped)
+	}
+
+	byName := map[string]ComponentPosture{}
+	for _, c := range doc.Predicate.Components {
+		byName[c.Name] = c
+	}
+
+	api := byName["kube-apiserver"]
+	if api.Result != ComponentResultPass || api.Successes != 1 || api.Skipped != 1 {
+		t.Errorf("kube-apiserver = %+v", api)
+	}
+	if len(api.Images) != 1 {
+		t.Errorf("kube-apiserver images = %v", api.Images)
+	}
+
+	ing := byName["openshift-ingress"]
+	if ing.Result != ComponentResultFail || ing.Failures != 1 {
+		t.Errorf("openshift-ingress = %+v", ing)
+	}
+	if len(ing.FailureDetails) != 1 || ing.FailureDetails[0].Port != 443 {
+		t.Errorf("openshift-ingress failureDetails = %+v", ing.FailureDetails)
+	}
+
+	unk := byName["unknown"]
+	if unk.Result != ComponentResultSkip {
+		t.Errorf("unknown = %+v", unk)
+	}
+}
+
+func TestBuildTLSPostureAttestationObserveMode(t *testing.T) {
+	t.Parallel()
+
+	results := scanner.ScanResults{
+		IPResults: []scanner.IPResult{{
+			IP:                 "10.0.0.1",
+			OpenshiftComponent: &k8s.OpenshiftComponent{Component: "foo"},
+			PortResults: []scanner.PortResult{{
+				Port:           443,
+				Status:         scanner.StatusOK,
+				TLS13Supported: false,
+				MLKEMSupported: false,
+			}},
+		}},
+	}
+
+	doc := BuildTLSPostureAttestation(results, false, AttestationMeta{ScannerVersion: "dev"})
+	if doc.Predicate.PolicyBar != PolicyBarObserve {
+		t.Errorf("PolicyBar = %q, want observe", doc.Predicate.PolicyBar)
+	}
+	if doc.Predicate.Result != OverallResultPass {
+		t.Errorf("observe mode Result = %q, want PASS", doc.Predicate.Result)
+	}
+	if doc.Predicate.Components[0].Result != ComponentResultPass {
+		t.Errorf("component result = %q, want PASS in observe mode", doc.Predicate.Components[0].Result)
+	}
+}
+
+func TestBuildTLSPostureAttestationTLSProfileBar(t *testing.T) {
+	t.Parallel()
+
+	results := scanner.ScanResults{
+		TLSSecurityConfig: &k8s.TLSSecurityProfile{
+			TLSAdherence: configv1.TLSAdherencePolicyStrictAllComponents,
+		},
+		IPResults: []scanner.IPResult{{
+			IP:                 "10.0.0.1",
+			OpenshiftComponent: &k8s.OpenshiftComponent{Component: "kube-apiserver"},
+			PortResults: []scanner.PortResult{{
+				Port:   6443,
+				Status: scanner.StatusOK,
+				APIServerTLSConfigCompliance: &scanner.TLSConfigComplianceResult{
+					Version: false,
+					Ciphers: true,
+				},
+			}},
+		}},
+	}
+
+	doc := BuildTLSPostureAttestation(results, false, AttestationMeta{})
+	if doc.Predicate.PolicyBar != PolicyBarTLSProfile {
+		t.Errorf("PolicyBar = %q, want tls-profile", doc.Predicate.PolicyBar)
+	}
+	if doc.Predicate.Result != OverallResultFail {
+		t.Errorf("Result = %q, want FAIL", doc.Predicate.Result)
+	}
+	if len(doc.Predicate.Components[0].FailureDetails) != 1 {
+		t.Fatalf("expected failure detail, got %+v", doc.Predicate.Components[0].FailureDetails)
+	}
+}

Relevance

⭐⭐ Medium

Table-driven conversion is maintainability/style; no clear historical enforcement signal in this
repo.

PR-#41

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 452 requires table-driven tests for multiple cases of the same function/behavior.
The added tests exercise BuildTLSPostureAttestation in multiple scenarios using separate,
repetitive test functions instead of a single test-case table iterated in a loop.

Rule 452: Use table-driven tests for all test cases in supported languages
internal/output/attestation_test.go[15-177]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`BuildTLSPostureAttestation` has multiple scenario tests implemented as separate functions (`PQCRollup`, `ObserveMode`, `TLSProfileBar`) rather than a single table-driven test with a cases slice and loop.

## Issue Context
Compliance requires table-driven tests when validating the same behavior across multiple input/output combinations.

## Fix Focus Areas
- internal/output/attestation_test.go[15-177]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. ClusterVersion GET lacks timeout 🐞 Bug ☼ Reliability

Description

GetClusterVersionInfo uses context.TODO() for the clusterversion/cluster GET, so there is no
application-level deadline and output generation can stall under API slowness/unresponsiveness.
Other cluster operations already use context.WithTimeout, so this call should similarly bound its
runtime.

Code

internal/k8s/clusterversion.go[R18-24]

+func (c *Client) GetClusterVersionInfo() (*ClusterVersionInfo, error) {
+	if c == nil || c.configClient == nil {
+		return nil, fmt.Errorf("openshift config client not available")
+	}
+	cv, err := c.configClient.ConfigV1().ClusterVersions().Get(context.TODO(), "cluster", metav1.GetOptions{})
+	if err != nil {
+		return nil, fmt.Errorf("get clusterversion/cluster: %w", err)

Relevance

⭐⭐ Medium

Timeouts for k8s calls are sensible, but no close precedent that all GETs must be deadline-bounded.

PR-#37

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

GetClusterVersionInfo performs the GET with context.TODO (no deadline), while other code paths
explicitly use context.WithTimeout for k8s calls, indicating expected bounded operations.

internal/k8s/clusterversion.go[16-24]
internal/k8s/discovery.go[106-118]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The ClusterVersion API request uses `context.TODO()` (no deadline/cancel), which can delay `writeOutputs` and artifact emission when the apiserver is slow or the connection hangs.

### Issue Context
The codebase already uses time-bounded contexts for other k8s operations (e.g., pod exec). ClusterVersion lookup should follow the same reliability pattern.

### Fix Focus Areas
- internal/k8s/clusterversion.go[18-24]
- internal/k8s/discovery.go[106-118]

### Suggested change
- Introduce a small timeout (e.g., 5–15s) in `GetClusterVersionInfo`:
 - `ctx, cancel := context.WithTimeout(context.Background(), <timeout>)`
 - `defer cancel()`
 - Use `ctx` in the `.Get(...)` call.
- (Optional) Make the timeout a constant in `internal/k8s` for consistency/testing.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Attestation errors not propagated 🐞 Bug ☼ Reliability

Description

WriteOutputFiles logs WriteAttestationFile failures but always returns nil, so tls-scanner can exit
successfully even when a requested --attestation-file was not written. main.go only fails the run
when WriteOutputFiles returns a non-nil error, so CI can proceed with missing attestations.

Code

internal/output/json.go[R77-86]

+	if attestationFile != "" {
+		attestationPath := resolveOutputPath(artifactDir, attestationFile)
+		attMeta := AttestationMeta{}
+		if meta != nil {
+			attMeta = *meta
+		}
+		if err := WriteAttestationFile(results, attestationPath, pqcCheck, attMeta); err != nil {
+			slog.Error("writing attestation output", "error", err)
+		}
+	}

Relevance

⭐⭐ Medium

Propagating attestation write failures changes tool exit semantics; could be desired for CI, but
might be debated.

PR-#41

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

WriteOutputFiles logs and ignores the error from WriteAttestationFile; main.go only exits non-zero
when writeOutputs/WriteOutputFiles returns an error, so a failed attestation write won’t affect the
exit code.

internal/output/json.go[77-88]
cmd/tls-scanner/main.go[226-232]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`WriteOutputFiles` swallows attestation write errors (logs them and returns `nil`), so callers cannot reliably enforce that a requested `--attestation-file` was produced.

### Issue Context
`cmd/tls-scanner/main.go` treats `WriteOutputFiles` errors as fatal, but today only directory creation errors are returned. For CI signing workflows, failure to write the attestation should be detectable via exit code.

### Fix Focus Areas
- internal/output/json.go[31-88]
- cmd/tls-scanner/main.go[226-232]

### Suggested change
- In `WriteOutputFiles`, if `attestationFile != ""` and `WriteAttestationFile(...)` returns an error, return that error (or accumulate and return a combined error if you want consistent behavior across JSON/CSV/JUnit/attestation).
- Update/extend tests to assert that `WriteOutputFiles` returns an error when attestation writing fails (e.g., unwritable directory).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

4. Unneeded ClusterVersion API call 🐞 Bug ➹ Performance

Description

In run(), writeOutputs calls client.GetClusterVersionInfo() whenever client != nil (i.e.,
--all-pods), even when --attestation-file is empty, adding an avoidable API request and potential
latency/failure. This happens even in cases where WriteOutputFiles would otherwise quickly no-op
based on empty output filenames.

Code

cmd/tls-scanner/main.go[R180-193]

+	writeOutputs := func(scanResults scanner.ScanResults) error {
+		meta := &output.AttestationMeta{
+			ScannerVersion: version,
+			ScannerCommit:  commit,
+		}
+		if client != nil {
+			if cv, err := client.GetClusterVersionInfo(); err != nil {
+				slog.Debug("cluster version unavailable for attestation", "error", err)
+			} else {
+				meta.ClusterVersion = cv
+			}
+		}
+		return output.WriteOutputFiles(scanResults, *artifactDir, *jsonFile, *csvFile, *junitFile, *attestationFile, isPQCCheck, meta)
+	}

Relevance

⭐⭐⭐ High

Repo accepted optimizations reducing unnecessary cluster/API work; gating ClusterVersion lookup on
attestation is low-risk.

PR-#59

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

writeOutputs unconditionally performs the ClusterVersion lookup when a client exists, regardless of
whether an attestation file is requested; WriteOutputFiles itself would no-op when all output
filenames are empty, but the lookup happens before that decision.

cmd/tls-scanner/main.go[180-193]
internal/output/json.go[31-34]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`client.GetClusterVersionInfo()` is called for every `--all-pods` run even when `--attestation-file` is not set, causing an unnecessary OpenShift API request.

### Issue Context
This lookup is only used to populate attestation metadata; it should be performed only when attestation output is actually requested.

### Fix Focus Areas
- cmd/tls-scanner/main.go[180-193]
- internal/output/json.go[31-34]

### Suggested change
- In `writeOutputs`, wrap the ClusterVersion lookup with `if *attestationFile != "" && client != nil { ... }`.
- Optionally, only allocate/populate `AttestationMeta` when `*attestationFile != ""` (still pass `nil` meta to `WriteOutputFiles` otherwise).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

• Author self-review: I have reviewed the code review findings, and addressed the relevant ones.

[openshift-ci]

@richardsonnick: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.