Skip to content

CNTRLPLANE-3423: e2e TLS test for service-operator-ca #31480

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kaleemsiddiqu wants to merge 1 commit into
base: main
Choose a base branch
from
+296 −0
Open

CNTRLPLANE-3423: e2e TLS test for service-operator-ca #31480

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
296 changes: 296 additions & 0 deletions test/extended/tls/tls_observed_config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -621,6 +621,188 @@ var _ = g.Describe("[sig-api-machinery][Feature:TLSObservedConfig][Serial][Disru
})
})

// ── Service CA operator TLS tests ─────────────────────────────────────
// These tests validate the service-ca-operator's centralized TLS security
// profile integration (openshift/service-ca-operator#365). The operator
// observes the APIServer CR's tlsSecurityProfile and propagates
// minTLSVersion and cipherSuites through its observedConfig into the
// service-ca-controller-config ConfigMap.

var _ = g.Describe("[sig-api-machinery][Feature:TLSObservedConfig][Serial][Suite:openshift/tls-observed-config] Service CA operator", g.Ordered, func() {
defer g.GinkgoRecover()

oc := exutil.NewCLI("tls-service-ca-operator")
ctx := context.Background()

g.BeforeAll(func() {
isMicroShift, err := exutil.IsMicroShiftCluster(oc.AdminKubeClient())
o.Expect(err).NotTo(o.HaveOccurred())
if isMicroShift {
g.Skip("Service CA operator TLS tests are not applicable to MicroShift clusters")
}

isHyperShift, err := exutil.IsHypershift(ctx, oc.AdminConfigClient())
o.Expect(err).NotTo(o.HaveOccurred())
if isHyperShift {
g.Skip("Service CA operator TLS tests are not applicable to HyperShift clusters")
}
})

g.It("should have TLS entries in the service-ca operator observedConfig", func() {
apiserverConfig, err := oc.AdminConfigClient().ConfigV1().APIServers().Get(ctx, "cluster", metav1.GetOptions{})
o.Expect(err).NotTo(o.HaveOccurred())
expected := captureTLSConfiguration(apiserverConfig.Spec.TLSSecurityProfile)

target := newObservedConfigTarget(
"openshift-service-ca-operator",
gvr("operator.openshift.io", "v1", "servicecas"),
"cluster",
[]string{"servingInfo"},
)
err = target.testTLS(oc, ctx, expected)
o.Expect(err).NotTo(o.HaveOccurred(),
"Service CA operator observedConfig should have TLS entries matching APIServer profile")
})

g.It("should have TLS entries in the service-ca-controller-config ConfigMap", func() {
apiserverConfig, err := oc.AdminConfigClient().ConfigV1().APIServers().Get(ctx, "cluster", metav1.GetOptions{})
o.Expect(err).NotTo(o.HaveOccurred())
expected := captureTLSConfiguration(apiserverConfig.Spec.TLSSecurityProfile)

err = verifyServiceCAConfigMap(oc, ctx, expected)
o.Expect(err).NotTo(o.HaveOccurred(),
"service-ca-controller-config ConfigMap should have TLS entries matching APIServer profile")
})
})

var _ = g.Describe("[sig-api-machinery][Feature:TLSObservedConfig][Serial][Disruptive][Suite:openshift/tls-observed-config] Service CA operator", g.Ordered, func() {
defer g.GinkgoRecover()

oc := exutil.NewCLI("tls-service-ca-operator-serial")
ctx := context.Background()

@coderabbitai coderabbitai Bot Aug 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Bound and enforce profile-restoration cleanup.

The cleanup defer uses context.Background() without a deadline and only logs failed service-ca reconciliation. Use context.WithoutCancel(configChangeCtx) with an explicit timeout, pass it to restoration and reconciliation, and fail the spec when reconciliation fails.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/extended/tls/tls_observed_config.go` at line 682, Update the cleanup
defer around configChangeCtx to derive a context with
context.WithoutCancel(configChangeCtx) and an explicit timeout, ensuring the
timeout is released. Pass this bounded context to profile restoration and
service-ca reconciliation, and fail the spec when reconciliation returns an
error instead of only logging it.


g.BeforeAll(func() {
isMicroShift, err := exutil.IsMicroShiftCluster(oc.AdminKubeClient())
o.Expect(err).NotTo(o.HaveOccurred())
if isMicroShift {
g.Skip("Service CA operator TLS tests are not applicable to MicroShift clusters")
}

isHyperShift, err := exutil.IsHypershift(ctx, oc.AdminConfigClient())
o.Expect(err).NotTo(o.HaveOccurred())
if isHyperShift {
g.Skip("Service CA operator TLS tests are not applicable to HyperShift clusters")
}
})

g.It("should enforce TLS versions through service-ca endpoint across profile changes [Timeout:60m]", func() {
configChangeCtx, configChangeCancel := context.WithTimeout(ctx, 60*time.Minute)
defer configChangeCancel()

g.By("saving original TLS profile for restoration")
originalAPIServer, err := oc.AdminConfigClient().ConfigV1().APIServers().Get(configChangeCtx, "cluster", metav1.GetOptions{})
o.Expect(err).NotTo(o.HaveOccurred())
originalProfile := originalAPIServer.Spec.TLSSecurityProfile

defer func() {
g.By("restoring original TLS profile")
setAPIServerTLSProfile(oc, ctx, originalProfile, "original")
err := exutil.WaitForOperatorProgressingFalse(ctx, oc.AdminConfigClient(), "service-ca")
if err != nil {
e2e.Logf("Warning: service-ca operator did not finish reconciling during cleanup: %v", err)
}
Comment on lines +707 to +713

@coderabbitai coderabbitai Bot Aug 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Wait for full Service CA restoration.

WaitForOperatorProgressingFalse can return before observedConfig and service-ca-controller-config converge. Cleanup can finish while Service CA still uses the profile from this test. A following serial test can then start from stale TLS state.

Capture the original tlsConfig before the first profile change. After setAPIServerTLSProfile, call waitForServiceCATLSConfig with that configuration before cleanup completes. Use the bounded cleanup context.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/extended/tls/tls_observed_config.go` around lines 707 - 713, Capture the
original tlsConfig before the first TLS profile change in the test. In the
cleanup defer after setAPIServerTLSProfile restores originalProfile, call
waitForServiceCATLSConfig with the captured configuration and bounded cleanup
context, then retain the existing operator-progress wait and warning handling.

}()

intermediateProfile := &configv1.TLSSecurityProfile{
Type: configv1.TLSProfileIntermediateType,
Intermediate: &configv1.IntermediateTLSProfile{},
}
modernProfile := &configv1.TLSSecurityProfile{
Type: configv1.TLSProfileModernType,
Modern: &configv1.ModernTLSProfile{},
}
intermediateTLSConfig := captureTLSConfiguration(intermediateProfile)
modernTLSConfig := captureTLSConfiguration(modernProfile)

// Step 1: Set Intermediate profile and verify config + wire-level TLS
g.By("setting TLS profile to Intermediate")
setAPIServerTLSProfile(oc, configChangeCtx, intermediateProfile, "Intermediate")

target := newObservedConfigTarget("openshift-service-ca-operator",
gvr("operator.openshift.io", "v1", "servicecas"), "cluster", []string{"servingInfo"})

g.By("waiting for service-ca operator to reconcile Intermediate TLS config")
err = waitForServiceCATLSConfig(oc, configChangeCtx, target, intermediateTLSConfig)
o.Expect(err).NotTo(o.HaveOccurred(), "service-ca should reconcile Intermediate profile")

g.By("verifying service-ca observedConfig has Intermediate TLS config")
err = target.testTLS(oc, configChangeCtx, intermediateTLSConfig)
o.Expect(err).NotTo(o.HaveOccurred(), "observedConfig should match Intermediate profile")

g.By("verifying service-ca-controller-config ConfigMap has Intermediate TLS config")
err = verifyServiceCAConfigMap(oc, configChangeCtx, intermediateTLSConfig)
o.Expect(err).NotTo(o.HaveOccurred(), "service-ca ConfigMap should match Intermediate profile")

g.By("verifying TLS 1.2 handshake works with Intermediate profile")
err = testServiceCAEndpointTLS(oc, configChangeCtx, tls.VersionTLS12, true)
o.Expect(err).NotTo(o.HaveOccurred(), "TLS 1.2 should work with Intermediate profile")

g.By("verifying TLS 1.3 handshake works with Intermediate profile")
err = testServiceCAEndpointTLS(oc, configChangeCtx, tls.VersionTLS13, true)
o.Expect(err).NotTo(o.HaveOccurred(), "TLS 1.3 should work with Intermediate profile")

// Step 2: Switch to Modern profile — only TLS 1.3 should work
g.By("switching to Modern TLS profile")
setAPIServerTLSProfile(oc, configChangeCtx, modernProfile, "Modern")

g.By("waiting for service-ca operator to reconcile Modern TLS config")
err = waitForServiceCATLSConfig(oc, configChangeCtx, target, modernTLSConfig)
o.Expect(err).NotTo(o.HaveOccurred(), "service-ca should reconcile Modern profile")

g.By("verifying service-ca observedConfig has Modern TLS config")
err = target.testTLS(oc, configChangeCtx, modernTLSConfig)
o.Expect(err).NotTo(o.HaveOccurred(), "observedConfig should match Modern profile")

g.By("verifying service-ca-controller-config ConfigMap has Modern TLS config")
err = verifyServiceCAConfigMap(oc, configChangeCtx, modernTLSConfig)
o.Expect(err).NotTo(o.HaveOccurred(), "service-ca ConfigMap should match Modern profile")

g.By("verifying TLS 1.3 handshake works with Modern profile")
err = testServiceCAEndpointTLS(oc, configChangeCtx, tls.VersionTLS13, true)
o.Expect(err).NotTo(o.HaveOccurred(), "TLS 1.3 should work with Modern profile")

g.By("verifying TLS 1.2 handshake is rejected with Modern profile")
err = testServiceCAEndpointTLS(oc, configChangeCtx, tls.VersionTLS12, false)
o.Expect(err).NotTo(o.HaveOccurred(), "TLS 1.2 should be rejected with Modern profile")

// Step 3: Downgrade back to Intermediate — both TLS 1.2 and 1.3 should work again
g.By("switching back to Intermediate TLS profile (downgrade from Modern)")
setAPIServerTLSProfile(oc, configChangeCtx, intermediateProfile, "Intermediate")

g.By("waiting for service-ca operator to reconcile Intermediate TLS config after downgrade")
err = waitForServiceCATLSConfig(oc, configChangeCtx, target, intermediateTLSConfig)
o.Expect(err).NotTo(o.HaveOccurred(), "service-ca should reconcile Intermediate profile after downgrade")

g.By("verifying service-ca observedConfig has Intermediate TLS config after downgrade")
err = target.testTLS(oc, configChangeCtx, intermediateTLSConfig)
o.Expect(err).NotTo(o.HaveOccurred(), "observedConfig should match Intermediate profile after downgrade")

g.By("verifying service-ca-controller-config ConfigMap has Intermediate TLS config after downgrade")
err = verifyServiceCAConfigMap(oc, configChangeCtx, intermediateTLSConfig)
o.Expect(err).NotTo(o.HaveOccurred(), "service-ca ConfigMap should match Intermediate profile after downgrade")

g.By("verifying TLS 1.2 handshake works after downgrade to Intermediate")
err = testServiceCAEndpointTLS(oc, configChangeCtx, tls.VersionTLS12, true)
o.Expect(err).NotTo(o.HaveOccurred(), "TLS 1.2 should work after downgrade to Intermediate")

g.By("verifying TLS 1.3 handshake works after downgrade to Intermediate")
err = testServiceCAEndpointTLS(oc, configChangeCtx, tls.VersionTLS13, true)
o.Expect(err).NotTo(o.HaveOccurred(), "TLS 1.3 should work after downgrade to Intermediate")

e2e.Logf("=== Service CA operator TLS profile change validation complete ===")
})
})

// ─── Test implementations ──────────────────────────────────────────────────

// verifyAllTLSConfiguration runs all TLS validation tests across all components
Expand Down Expand Up @@ -1783,3 +1965,117 @@ func setupHyperShiftManagement() (*exutil.CLI, string, string, error) {

return mgmtOC, hcpNamespace, hostedClusterConfigName, nil
}

// ─── Service CA operator helpers ─────────────────────────────────────────

// waitForServiceCATLSConfig polls the service-ca operator's observedConfig and
// its controller ConfigMap until both reflect the expected TLS configuration.
// WaitForOperatorProgressingFalse alone is insufficient because the operator can
// report Progressing=False before the config observer writes the new values.
func waitForServiceCATLSConfig(oc *exutil.CLI, ctx context.Context, target observedConfigTarget, expected tlsConfig) error {
err := exutil.WaitForOperatorProgressingFalse(ctx, oc.AdminConfigClient(), "service-ca")
if err != nil {
return fmt.Errorf("service-ca operator did not finish reconciling: %w", err)
}

err = wait.PollUntilContextTimeout(ctx, 10*time.Second, 5*time.Minute, true,
func(ctx context.Context) (bool, error) {
if err := target.testTLS(oc, ctx, expected); err != nil {
e2e.Logf(" poll: observedConfig not yet updated: %v", err)
return false, nil
}
if err := verifyServiceCAConfigMap(oc, ctx, expected); err != nil {
e2e.Logf(" poll: ConfigMap not yet updated: %v", err)
return false, nil
}
return true, nil
})
if err != nil {
return fmt.Errorf("service-ca TLS config did not converge to %s profile: %w", expected.profileType, err)
}
return nil
}

// verifyServiceCAConfigMap fetches the service-ca-controller-config ConfigMap
// and validates that its servingInfo section matches the expected TLS configuration.
func verifyServiceCAConfigMap(oc *exutil.CLI, ctx context.Context, expected tlsConfig) error {
cm, err := oc.AdminKubeClient().CoreV1().ConfigMaps("openshift-service-ca").Get(ctx, "service-ca-controller-config", metav1.GetOptions{})
if err != nil {
return fmt.Errorf("failed to get service-ca-controller-config ConfigMap: %w", err)
}

configData, found := cm.Data["controller-config.yaml"]
if !found {
return fmt.Errorf("service-ca-controller-config ConfigMap is missing 'controller-config.yaml' key")
}

var configObj map[string]interface{}
if err := yaml.Unmarshal([]byte(configData), &configObj); err != nil {
return fmt.Errorf("failed to parse controller-config.yaml: %w", err)
}

return validateServingInfoTLSConfig(oc, ctx, configObj, []string{"servingInfo"}, expected)
}

// testServiceCAEndpointTLS performs a single TLS connection attempt against
// the service-ca controller pod on port 8443 via port-forward.
func testServiceCAEndpointTLS(oc *exutil.CLI, ctx context.Context, tlsVersion uint16, shouldSucceed bool) error {
deployment, err := oc.AdminKubeClient().AppsV1().Deployments("openshift-service-ca").Get(ctx, "service-ca", metav1.GetOptions{})
if err != nil {
return fmt.Errorf("failed to get service-ca deployment: %w", err)
}

selectorString := labels.Set(deployment.Spec.Selector.MatchLabels).String()
podList, err := oc.AdminKubeClient().CoreV1().Pods("openshift-service-ca").List(ctx, metav1.ListOptions{
LabelSelector: selectorString,
})
if err != nil {
return fmt.Errorf("failed to list service-ca pods: %w", err)
}

var runningPod string
for _, pod := range podList.Items {
if pod.Status.Phase == corev1.PodRunning {
runningPod = pod.Name
break
}
}
if runningPod == "" {
return fmt.Errorf("no running service-ca pods found")
}

resourceName := fmt.Sprintf("pod/%s", runningPod)
return forwardPortAndExecute(oc, resourceName, "openshift-service-ca", "8443",
func(localPort int) error {
tlsConf := &tls.Config{
MinVersion: tlsVersion,
MaxVersion: tlsVersion,
InsecureSkipVerify: true,
}

dialer := &net.Dialer{Timeout: 10 * time.Second}
conn, err := tls.DialWithDialer(dialer, "tcp", fmt.Sprintf("127.0.0.1:%d", localPort), tlsConf)

versionName := tlsVersionName(tlsVersion)
if shouldSucceed {
if err != nil {
return fmt.Errorf("service-ca endpoint: TLS %s connection should succeed but failed: %v", versionName, err)
}
negotiated := conn.ConnectionState().Version
conn.Close()
Comment on lines +2064 to +2065

@coderabbitai coderabbitai Bot Aug 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -eu
file=$(fd -t f 'tls_observed_config\.go$' . | head -n 1)
printf '%s\n' "FILE=$file"
sed -n '1995,2060p' "$file"
printf '%s\n' '--- Close calls in file ---'
rg -n -C 3 '\.Close\(\)' "$file"
printf '%s\n' '--- surrounding function outline ---'
ast-grep outline "$file" | sed -n '1,160p'

Repository: openshift/origin

Length of output: 10243

🏁 Script executed:

#!/bin/bash
set -eu
file=$(fd -t f 'tls_observed_config\.go$' . | head -n 1)
python3 - "$file" <<'PY'
import re, sys
from pathlib import Path
p = Path(sys.argv[1])
lines = p.read_text().splitlines()
for i, line in enumerate(lines):
    if '.Close()' in line:
        start=max(0,i-8); end=min(len(lines),i+9)
        print(f'--- lines {start+1}-{end} ---')
        for n in range(start,end):
            print(f'{n+1:5}: {lines[n]}')
PY

Repository: openshift/origin

Length of output: 4071

Handle tls.Conn.Close errors.

Both paths discard the error from conn.Close(). Return a wrapped error, or log it under a defined non-fatal policy.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/extended/tls/tls_observed_config.go` around lines 2035 - 2036, Update
both paths around the tls.Conn cleanup in the relevant test flow to handle the
error returned by conn.Close() instead of discarding it. Propagate it as a
wrapped error when cleanup failure should fail the operation, or apply the
project’s established non-fatal logging policy if cleanup is best effort;
preserve the existing negotiated value handling.

Source: Path instructions

e2e.Logf("Service CA endpoint: TLS %s connection succeeded (negotiated %s)", versionName, tlsVersionName(negotiated))
return nil
}

if err == nil {
negotiated := conn.ConnectionState().Version
conn.Close()
return fmt.Errorf("service-ca endpoint: TLS %s connection should fail but succeeded (negotiated %s)",
versionName, tlsVersionName(negotiated))
}

e2e.Logf("Service CA endpoint: TLS %s connection rejected as expected: %v", versionName, err)
return nil
Comment on lines +2076 to +2078

@coderabbitai coderabbitai Bot Aug 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Reject only TLS-version handshake failures.

When shouldSucceed is false, Lines 2047-2049 convert every dial error into success. A broken port-forward or unavailable endpoint can pass the Modern-profile TLS 1.2 rejection check without a TLS version rejection.

Accept only the TLS rejection errors already classified by checkTLSConnection. Return every other error.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/extended/tls/tls_observed_config.go` around lines 2047 - 2049, Update
the shouldSucceed=false branch in checkTLSConnection to return success only when
the dial error is classified as a TLS-version rejection by the existing
checkTLSConnection logic; propagate any other error, including unavailable
endpoints or broken port-forwards, instead of unconditionally returning nil
after logging.

},
)
}