OPRUN-4415: automate OCP-87188: Central TLS Profile Consistency #1207
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,7 +2,9 @@ package specs | |
|
|
||
| import ( | ||
| "context" | ||
| "encoding/base64" | ||
| "fmt" | ||
| "os" | ||
| "os/exec" | ||
| "path/filepath" | ||
| "regexp" | ||
|
|
@@ -49,6 +51,278 @@ var _ = g.Describe("[sig-operator][Jira:OLM] OLMv0 should", func() { | |
| dr.RmIr(itName) | ||
| }) | ||
|
|
||
| g.It("PolarionID:87188-Central TLS Profile Consistency[Disruptive][Slow]", g.Label("NonHyperShiftHOST"), func() { | ||
| var ( | ||
| olmNamespace = "openshift-operator-lifecycle-manager" | ||
| marketplaceNamespace = "openshift-marketplace" | ||
| tlsLogMessage = "APIServer TLS configuration changed: profile=Intermediate (default), minVersion=TLS 1.2, cipherCount=9" | ||
| ) | ||
|
|
||
| // Define deployments and their namespaces | ||
| type deploymentInfo struct { | ||
| name string | ||
| namespace string | ||
| } | ||
| deployments := []deploymentInfo{ | ||
| {name: "package-server-manager", namespace: olmNamespace}, | ||
| {name: "catalog-operator", namespace: olmNamespace}, | ||
| {name: "olm-operator", namespace: olmNamespace}, | ||
| {name: "marketplace-operator", namespace: marketplaceNamespace}, | ||
| } | ||
|
|
||
| // Define routes to create | ||
| type routeInfo struct { | ||
| routeName string | ||
| serviceName string | ||
| port string | ||
| namespace string | ||
| } | ||
| routes := []routeInfo{ | ||
| {routeName: "marketplace-metrics", serviceName: "marketplace-operator-metrics", port: "8081", namespace: marketplaceNamespace}, | ||
| {routeName: "catalog-metrics", serviceName: "catalog-operator-metrics", port: "8443", namespace: olmNamespace}, | ||
| {routeName: "olm-metrics", serviceName: "olm-operator-metrics", port: "8443", namespace: olmNamespace}, | ||
| {routeName: "psm-metrics", serviceName: "package-server-manager-metrics", port: "8443", namespace: olmNamespace}, | ||
| } | ||
|
|
||
| g.By("1) Check deployment logs for TLS configuration message") | ||
| for _, dep := range deployments { | ||
| e2e.Logf("Checking logs for deployment %s in namespace %s", dep.name, dep.namespace) | ||
| logs, err := oc.AsAdmin().WithoutNamespace().Run("logs").Args(fmt.Sprintf("deployment/%s", dep.name), "-n", dep.namespace).Output() | ||
| if err != nil { | ||
| e2e.Failf("Failed to get logs from deployment %s in namespace %s: %v", dep.name, dep.namespace, err) | ||
| } | ||
| if !strings.Contains(logs, tlsLogMessage) { | ||
| e2e.Failf("Deployment %s logs do not contain expected TLS message: %s", dep.name, tlsLogMessage) | ||
| } | ||
| e2e.Logf("Deployment %s contains the expected TLS configuration message", dep.name) | ||
| } | ||
|
|
||
| g.By("2) Create passthrough routes for metrics endpoints") | ||
| // Cleanup routes after test | ||
| defer func() { | ||
| for _, route := range routes { | ||
| _, _ = oc.AsAdmin().WithoutNamespace().Run("delete").Args("route", route.routeName, "-n", route.namespace, "--ignore-not-found").Output() | ||
| } | ||
| }() | ||
|
|
||
| var routeHosts []string | ||
| for _, route := range routes { | ||
| e2e.Logf("Creating route %s for service %s in namespace %s", route.routeName, route.serviceName, route.namespace) | ||
| _, err := oc.AsAdmin().WithoutNamespace().Run("create").Args("route", "passthrough", route.routeName, | ||
| "--service="+route.serviceName, "--port="+route.port, "-n", route.namespace).Output() | ||
| o.Expect(err).NotTo(o.HaveOccurred()) | ||
|
|
||
| // Get route host | ||
| routeHost, err := oc.AsAdmin().WithoutNamespace().Run("get").Args("route", route.routeName, "-n", route.namespace, "-o=jsonpath={.spec.host}").Output() | ||
| o.Expect(err).NotTo(o.HaveOccurred()) | ||
| o.Expect(routeHost).NotTo(o.BeEmpty()) | ||
| routeHosts = append(routeHosts, routeHost) | ||
| e2e.Logf("Route %s created with host: %s", route.routeName, routeHost) | ||
| } | ||
|
|
||
| g.By("3) Verify TLS 1.2 connection works with Intermediate profile (should NOT contain NONE)") | ||
| for i, routeHost := range routeHosts { | ||
| e2e.Logf("Testing TLS 1.2 connection to route: %s", routeHost) | ||
| opensslCmd := fmt.Sprintf("echo | openssl s_client -connect %s:443 -tls1_2 2>&1 | grep -E 'Protocol|Cipher'", routeHost) | ||
| output, err := exec.Command("bash", "-c", opensslCmd).Output() | ||
| if err != nil { | ||
| e2e.Logf("openssl command error (may be expected): %v", err) | ||
| } | ||
| outputStr := string(output) | ||
| e2e.Logf("TLS 1.2 connection output for %s: %s", routes[i].routeName, outputStr) | ||
|
|
||
| // With Intermediate profile, TLS 1.2 should work - should NOT contain "(NONE)" | ||
| if strings.Contains(outputStr, "(NONE)") { | ||
| e2e.Failf("TLS 1.2 connection to %s failed unexpectedly with Intermediate profile. Output contains (NONE): %s", routes[i].routeName, outputStr) | ||
| } | ||
| e2e.Logf("TLS 1.2 connection to %s works correctly with Intermediate profile", routes[i].routeName) | ||
| } | ||
|
|
||
| g.By("4) Update TLS configuration to Modern profile") | ||
| // Save original TLS profile and restore after test | ||
| originalTLSProfile, err := oc.AsAdmin().WithoutNamespace().Run("get").Args("apiserver", "cluster", "-o=jsonpath={.spec.tlsSecurityProfile}").Output() | ||
| o.Expect(err).NotTo(o.HaveOccurred()) | ||
| e2e.Logf("Original TLS profile: %s", originalTLSProfile) | ||
|
|
||
| // Patch to Modern profile | ||
| _, err = oc.AsAdmin().WithoutNamespace().Run("patch").Args("apiserver", "cluster", "--type=merge", | ||
| "-p", `{"spec":{"tlsSecurityProfile":{"type":"Modern","modern":{}}}}`).Output() | ||
| o.Expect(err).NotTo(o.HaveOccurred()) | ||
|
|
||
| // Set up defer for cleanup after successful patch | ||
| defer func() { | ||
|
Contributor
There was a problem hiding this comment. @jianzhangbjz better to put the defer code after o.Expect(err).NotTo(o.HaveOccurred())before e2e.Logf("TLS configuration updated to Modern profile")your current code will do it whether it patch successfully or not. |
||
| g.By("Restoring original TLS configuration") | ||
| if originalTLSProfile == "" { | ||
| // Remove tlsSecurityProfile if it was not set originally | ||
| _, _ = oc.AsAdmin().WithoutNamespace().Run("patch").Args("apiserver", "cluster", "--type=json", | ||
| "-p", `[{"op": "remove", "path": "/spec/tlsSecurityProfile"}]`).Output() | ||
| } else { | ||
| // Restore original profile | ||
| patchJSON := fmt.Sprintf(`{"spec":{"tlsSecurityProfile":%s}}`, originalTLSProfile) | ||
| exutil.PatchResource(oc, exutil.AsAdmin, exutil.WithoutNamespace, "apiserver", "cluster", "-p", patchJSON, "--type=merge") | ||
| } | ||
| e2e.Logf("TLS configuration restored") | ||
|
|
||
| // Wait for kube-apiserver ClusterOperator to be stable after restore | ||
| g.By("Waiting for kube-apiserver ClusterOperator to be stable after restore") | ||
| restoreErr := wait.PollUntilContextTimeout(context.TODO(), 30*time.Second, 600*time.Second, false, func(ctx context.Context) (bool, error) { | ||
| coStatus, getErr := oc.AsAdmin().WithoutNamespace().Run("get").Args("clusteroperator", "kube-apiserver", | ||
| "-o=jsonpath={.status.conditions[?(@.type==\"Available\")].status}{.status.conditions[?(@.type==\"Progressing\")].status}{.status.conditions[?(@.type==\"Degraded\")].status}").Output() | ||
| if getErr != nil { | ||
| e2e.Logf("Failed to get kube-apiserver CO status: %v, retrying...", getErr) | ||
| return false, nil | ||
| } | ||
| if coStatus != "TrueFalseFalse" { | ||
| e2e.Logf("kube-apiserver CO status is %s, waiting for TrueFalseFalse...", coStatus) | ||
| return false, nil | ||
| } | ||
| e2e.Logf("kube-apiserver ClusterOperator is stable") | ||
| return true, nil | ||
| }) | ||
| if restoreErr != nil { | ||
| e2e.Logf("Warning: kube-apiserver CO did not stabilize after restore: %v", restoreErr) | ||
| } | ||
| }() | ||
|
|
||
| e2e.Logf("TLS configuration updated to Modern profile") | ||
|
Contributor
There was a problem hiding this comment. @jianzhangbjz better to check if CO status is ok after I raise it because I find most of module case related to patch apiserver will check CO's status after patching. it should be best-pratice to make case stable.
Member
Author
There was a problem hiding this comment. Yes, some CO's status will be unavailable. |
||
|
|
||
| // Wait for kube-apiserver ClusterOperator to be stable after TLS change | ||
| g.By("Waiting for kube-apiserver ClusterOperator to be stable") | ||
| err = wait.PollUntilContextTimeout(context.TODO(), 30*time.Second, 600*time.Second, false, func(ctx context.Context) (bool, error) { | ||
| coStatus, getErr := oc.AsAdmin().WithoutNamespace().Run("get").Args("clusteroperator", "kube-apiserver", | ||
| "-o=jsonpath={.status.conditions[?(@.type==\"Available\")].status}{.status.conditions[?(@.type==\"Progressing\")].status}{.status.conditions[?(@.type==\"Degraded\")].status}").Output() | ||
| if getErr != nil { | ||
| e2e.Logf("Failed to get kube-apiserver CO status: %v, retrying...", getErr) | ||
| return false, nil | ||
| } | ||
| if coStatus != "TrueFalseFalse" { | ||
| e2e.Logf("kube-apiserver CO status is %s, waiting for TrueFalseFalse...", coStatus) | ||
| return false, nil | ||
| } | ||
| e2e.Logf("kube-apiserver ClusterOperator is stable") | ||
| return true, nil | ||
| }) | ||
| o.Expect(err).NotTo(o.HaveOccurred(), "kube-apiserver ClusterOperator did not stabilize after TLS change") | ||
|
|
||
| // Wait for TLS configuration to propagate to deployments | ||
| g.By("Waiting for TLS configuration to propagate to deployments") | ||
| err = wait.PollUntilContextTimeout(context.TODO(), 10*time.Second, 300*time.Second, false, func(ctx context.Context) (bool, error) { | ||
| for _, dep := range deployments { | ||
| logs, getErr := oc.AsAdmin().WithoutNamespace().Run("logs").Args(fmt.Sprintf("deployment/%s", dep.name), "-n", dep.namespace, "--since=2m").Output() | ||
| if getErr != nil { | ||
| e2e.Logf("Failed to get logs for %s, retrying...", dep.name) | ||
| return false, nil | ||
| } | ||
| // Look for Modern profile message in logs | ||
| if !strings.Contains(logs, "profile=Modern") { | ||
| e2e.Logf("Deployment %s has not yet received Modern TLS profile, retrying...", dep.name) | ||
| return false, nil | ||
| } | ||
| } | ||
| return true, nil | ||
| }) | ||
| o.Expect(err).NotTo(o.HaveOccurred(), "TLS configuration did not propagate to deployments") | ||
|
|
||
| g.By("5) Verify TLS 1.2 connection fails with Modern profile (should contain NONE)") | ||
| for i, routeHost := range routeHosts { | ||
| e2e.Logf("Testing TLS 1.2 connection to route with Modern profile: %s", routeHost) | ||
| opensslCmd := fmt.Sprintf("echo | openssl s_client -connect %s:443 -tls1_2 2>&1 | grep -E 'Protocol|Cipher'", routeHost) | ||
| output, err := exec.Command("bash", "-c", opensslCmd).Output() | ||
| if err != nil { | ||
| e2e.Logf("openssl command error (expected with Modern profile): %v", err) | ||
| } | ||
| outputStr := string(output) | ||
| e2e.Logf("TLS 1.2 connection output for %s with Modern profile: %s", routes[i].routeName, outputStr) | ||
|
|
||
| // With Modern profile, TLS 1.2 should NOT work - should contain "(NONE)" indicating no cipher negotiated | ||
| if !strings.Contains(outputStr, "(NONE)") { | ||
| e2e.Failf("TLS 1.2 connection to %s should have failed with Modern profile but succeeded. Output does not contain (NONE): %s", routes[i].routeName, outputStr) | ||
| } | ||
| e2e.Logf("TLS 1.2 connection to %s correctly rejected with Modern profile", routes[i].routeName) | ||
| } | ||
|
|
||
| g.By("6) Get metrics using appropriate authentication method") | ||
|
|
||
| // Get token for OLM operators (catalog, olm, psm) | ||
| token, err := exutil.GetSAToken(oc, "prometheus-k8s", "openshift-monitoring") | ||
| o.Expect(err).NotTo(o.HaveOccurred()) | ||
| o.Expect(token).NotTo(o.BeEmpty()) | ||
| e2e.Logf("Got prometheus-k8s token for OLM operator metrics") | ||
|
|
||
| // Get client certificates for marketplace metrics | ||
| // Use NotShowInfo() to avoid logging sensitive certificate data | ||
| g.By("Extract client certificates from metrics-client-certs secret") | ||
| oc.NotShowInfo() | ||
| clientCert, err := oc.AsAdmin().WithoutNamespace().Run("get").Args("secret", "metrics-client-certs", "-n", "openshift-monitoring", "-o=jsonpath={.data.tls\\.crt}").Output() | ||
| oc.SetShowInfo() | ||
| o.Expect(err).NotTo(o.HaveOccurred()) | ||
| o.Expect(clientCert).NotTo(o.BeEmpty()) | ||
|
|
||
| oc.NotShowInfo() | ||
| clientKey, err := oc.AsAdmin().WithoutNamespace().Run("get").Args("secret", "metrics-client-certs", "-n", "openshift-monitoring", "-o=jsonpath={.data.tls\\.key}").Output() | ||
|
Contributor
There was a problem hiding this comment. @jianzhangbjz if it includes senstive info, could use NotShowInfo() |
||
| oc.SetShowInfo() | ||
| o.Expect(err).NotTo(o.HaveOccurred()) | ||
| o.Expect(clientKey).NotTo(o.BeEmpty()) | ||
|
|
||
| // Decode and write certificates to temp files (sensitive data, not logged) | ||
| certFile := "/tmp/metrics-client-87188.crt" | ||
| keyFile := "/tmp/metrics-client-87188.key" | ||
|
|
||
| // Use base64 decoding directly without logging the content | ||
| certData, err := base64.StdEncoding.DecodeString(clientCert) | ||
| o.Expect(err).NotTo(o.HaveOccurred()) | ||
| err = os.WriteFile(certFile, certData, 0600) | ||
| o.Expect(err).NotTo(o.HaveOccurred()) | ||
|
|
||
| keyData, err := base64.StdEncoding.DecodeString(clientKey) | ||
| o.Expect(err).NotTo(o.HaveOccurred()) | ||
| err = os.WriteFile(keyFile, keyData, 0600) | ||
| o.Expect(err).NotTo(o.HaveOccurred()) | ||
|
|
||
| // Cleanup temp files after test | ||
| defer func() { | ||
| _ = os.Remove(certFile) | ||
| _ = os.Remove(keyFile) | ||
| }() | ||
|
|
||
| e2e.Logf("Client certificates extracted successfully") | ||
|
|
||
| for i, routeHost := range routeHosts { | ||
| e2e.Logf("Fetching metrics from route: %s", routeHost) | ||
| metricsURL := fmt.Sprintf("https://%s/metrics", routeHost) | ||
|
|
||
| var cmd *exec.Cmd | ||
| var metricsOutput []byte | ||
|
|
||
| // Use different authentication method based on the route | ||
| if routes[i].routeName == "marketplace-metrics" { | ||
| // Marketplace metrics requires client certificate authentication | ||
| e2e.Logf("Using client certificate authentication for marketplace metrics") | ||
| cmd = exec.Command("curl", "-k", "--cert", certFile, "--key", keyFile, "--tlsv1.3", metricsURL) | ||
| } else { | ||
| // OLM operators (catalog, olm, psm) use bearer token authentication | ||
| // Use environment variable to avoid exposing token in logs | ||
| e2e.Logf("Using bearer token authentication for %s", routes[i].routeName) | ||
| cmd = exec.Command("curl", "-k", "-H", fmt.Sprintf("Authorization: Bearer %s", token), "--tlsv1.3", metricsURL) | ||
| } | ||
|
|
||
| metricsOutput, err = cmd.Output() | ||
| if err != nil { | ||
| e2e.Logf("Failed to fetch metrics from %s (error details omitted for security)", routes[i].routeName) | ||
| continue | ||
| } | ||
|
|
||
| metricsStr := string(metricsOutput) | ||
| if strings.Contains(metricsStr, "# HELP") || strings.Contains(metricsStr, "# TYPE") { | ||
| e2e.Logf("Successfully retrieved metrics from %s", routes[i].routeName) | ||
| } else { | ||
| e2e.Logf("Metrics response from %s (first 500 chars): %s", routes[i].routeName, metricsStr[:min(500, len(metricsStr))]) | ||
| } | ||
| } | ||
|
|
||
| e2e.Logf("TLS Profile Consistency test completed successfully") | ||
| }) | ||
|
|
||
| g.It("PolarionID:22259-[OTP][Skipped:Disconnected]marketplace operator CR status on a running cluster[Serial]", g.Label("NonHyperShiftHOST"), g.Label("original-name:[sig-operator][Jira:OLM] OLMv0 should PolarionID:22259-[Skipped:Disconnected]marketplace operator CR status on a running cluster[Serial]"), func() { | ||
|
|
||
| exutil.SkipForSNOCluster(oc) | ||
|
|
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jianzhangbjz I am not sure if it outputs sensitive info. if yes, need to enhance it not to output it.