-
Notifications
You must be signed in to change notification settings - Fork 81
WIP: tls setup #643
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.
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
base: main
Are you sure you want to change the base?
WIP: tls setup #643
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,11 +2,13 @@ package tls | |
|
|
||
| import ( | ||
| "context" | ||
| "crypto/tls" | ||
| "fmt" | ||
| "strings" | ||
|
|
||
| configv1 "github.com/openshift/api/config/v1" | ||
| configlistersv1 "github.com/openshift/client-go/config/listers/config/v1" | ||
| crcommon "github.com/openshift/controller-runtime-common/pkg/tls" | ||
| libapiserver "github.com/openshift/library-go/pkg/operator/configobserver/apiserver" | ||
| libevents "github.com/openshift/library-go/pkg/operator/events" | ||
| "github.com/openshift/library-go/pkg/operator/resourcesynccontroller" | ||
|
|
@@ -93,3 +95,51 @@ func (a *apiServerListers) ResourceSyncer() resourcesynccontroller.ResourceSynce | |
| func (a *apiServerListers) PreRunHasSynced() []cache.InformerSynced { | ||
| return nil | ||
| } | ||
|
|
||
| // GetTLSConfigFromProfile returns a function that configures a tls.Config based on | ||
| // the provided TLS profile spec. The returned function is suitable for use with | ||
| // controller-runtime's metricsserver.Options TLSOpts field. | ||
| // Returns an error if the profile contains unsupported cipher suites. | ||
| func GetTLSConfigFromProfile(profileSpec configv1.TLSProfileSpec) (func(*tls.Config), error) { | ||
| configFn, unsupportedCiphers := crcommon.NewTLSConfigFromProfile(profileSpec) | ||
| if len(unsupportedCiphers) > 0 { | ||
| return nil, fmt.Errorf("TLS profile contains unsupported cipher suites: %v", unsupportedCiphers) | ||
| } | ||
| return configFn, nil | ||
| } | ||
|
|
||
| // ValidateMetricsAccess validates TLS configuration based on the cluster's adherence policy. | ||
| // In StrictAllComponents mode, returns an error if TLS profile cannot be fetched or is not configured. | ||
| // In LegacyAdheringComponentsOnly or NoOpinion mode, allows operation without TLS profile. | ||
| func ValidateMetricsAccess(ctx context.Context, c client.Client) error { | ||
| adherence, err := crcommon.FetchAPIServerTLSAdherencePolicy(ctx, c) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to fetch TLS adherence policy: %w", err) | ||
| } | ||
|
|
||
| // Only enforce in StrictAllComponents mode | ||
| if adherence == configv1.TLSAdherencePolicyStrictAllComponents { | ||
|
Comment on lines
+120
to
+121
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '--- pkg/tls/tlsprofile.go (relevant section) ---\n'
sed -n '1,220p' pkg/tls/tlsprofile.go | cat -n | sed -n '1,220p'
printf '\n--- search adherence policy usage ---\n'
rg -n "TLSAdherencePolicy|StrictAllComponents|adherence" -S .
printf '\n--- locate configv1 type definition in repo ---\n'
rg -n "type TLSAdherencePolicy|StrictAllComponents" -S pkg vendor api .Repository: openshift/local-storage-operator Length of output: 261 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Focus on the exact file and nearby code paths.
sed -n '1,220p' pkg/tls/tlsprofile.go | cat -nRepository: openshift/local-storage-operator Length of output: 7151 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Look up the upstream API docs and enum definition if not vendored locally.
python3 - <<'PY'
import sys, json, urllib.request
url = "https://pkg.go.dev/github.com/openshift/api/config/v1?tab=doc"
try:
with urllib.request.urlopen(url, timeout=15) as r:
data = r.read().decode('utf-8', 'replace')
print("fetched", len(data))
for needle in ["TLSAdherencePolicyStrictAllComponents", "TLSAdherencePolicy"]:
print(needle, needle in data)
except Exception as e:
print("ERR", repr(e))
PYRepository: openshift/local-storage-operator Length of output: 334 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Search the repository for the TLS adherence policy enum and call sites.
rg -n "TLSAdherencePolicyStrictAllComponents|TLSAdherencePolicy" -S .Repository: openshift/local-storage-operator Length of output: 4787 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Read the file in question with line numbers.
cat -n pkg/tls/tlsprofile.go | sed -n '1,220p'Repository: openshift/local-storage-operator Length of output: 7151 🌐 Web query:
💡 Result: In the OpenShift API (config/v1), the TLSAdherencePolicy field is validated using a Kubernetes custom resource validation enum [1][2][3]. The valid values for this field are restricted to "LegacyAdheringComponentsOnly" and "StrictAllComponents" [1][4]. To ensure forward compatibility, OpenShift components are designed to handle unknown values for the tlsAdherence field as follows [1][4]: 1. Defaulting Behavior: If a component encounters an unknown TLS adherence policy value, it must treat the setting as "StrictAllComponents" [1][4]. 2. Observability: Components are expected to log a warning when an unknown value is encountered, providing visibility into the configuration mismatch while defaulting to the more secure behavior [1][4][5]. This approach ensures that if new, stricter policies are added to the API in the future, existing components will default to the most secure state rather than falling back to potentially insecure legacy configurations [1][4]. Internal utility functions, such as those found in openshift/library-go, have been updated to support this pattern, including the addition of logging mechanisms for handling unrecognized enum values [5]. Citations:
🏁 Script executed: #!/bin/bash
set -euo pipefail
sed -n '1,220p' vendor/github.com/openshift/controller-runtime-common/pkg/tls/tls.go | cat -n
printf '\n---\n'
sed -n '1,220p' vendor/github.com/openshift/library-go/pkg/crypto/tls_adherence.go | cat -n
printf '\n---\n'
sed -n '270,320p' vendor/github.com/openshift/api/config/v1/types_apiserver.go | cat -nRepository: openshift/local-storage-operator Length of output: 11537 Treat unknown adherence policies as strict. 🤖 Prompt for AI AgentsSources: Path instructions, MCP tools |
||
| profileSpec, err := crcommon.FetchAPIServerTLSProfile(ctx, c) | ||
| if err != nil { | ||
| return fmt.Errorf("strict mode requires valid TLS profile: %w", err) | ||
| } | ||
| // Check if profile has any configuration | ||
| if profileSpec.Ciphers == nil && profileSpec.MinTLSVersion == "" { | ||
| return fmt.Errorf("strict mode requires explicit TLS profile") | ||
|
Comment on lines
+122
to
+128
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf 'Repo files of interest:\n'
git ls-files 'pkg/tls/*' 'pkg/**/tls*' | sed -n '1,120p'
printf '\nOutline for pkg/tls/tlsprofile.go:\n'
ast-grep outline pkg/tls/tlsprofile.go --view expanded || true
printf '\nRelevant lines from pkg/tls/tlsprofile.go:\n'
sed -n '1,220p' pkg/tls/tlsprofile.go
printf '\nSearch for FetchAPIServerTLSProfile definitions/usages:\n'
rg -n "FetchAPIServerTLSProfile|APIServerTLSProfile" -S .
printf '\nIf helper exists in vendored/internal path, show its file:\n'
fd -a "tls.go" . | sed -n '1,120p'Repository: openshift/local-storage-operator Length of output: 9164 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '--- pkg/tls/tlsprofile.go ---'
sed -n '1,220p' pkg/tls/tlsprofile.go
echo
echo '--- references in repo ---'
rg -n "FetchAPIServerTLSProfile|strict mode requires explicit TLS profile|explicit TLS profile|APIServerTLSProfile" pkg vendor . --glob '!**/node_modules/**' || true
echo
echo '--- attempt to inspect referenced upstream helper if network is available ---'
python3 - <<'PY'
import urllib.request
url = "https://raw.githubusercontent.com/openshift/controller-runtime-common/fea68df23430/pkg/tls/tls.go"
try:
with urllib.request.urlopen(url, timeout=10) as r:
data = r.read().decode("utf-8", "replace")
for i, line in enumerate(data.splitlines(), 1):
if "FetchAPIServerTLSProfile" in line or "TLSProfile" in line:
start = max(1, i-25)
end = min(len(data.splitlines()), i+80)
for j in range(start, end + 1):
print(f"{j:4d}: {data.splitlines()[j-1]}")
break
except Exception as e:
print(f"NETWORK_ERROR: {e}")
PYRepository: openshift/local-storage-operator Length of output: 8355 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '--- vendor/github.com/openshift/controller-runtime-common/pkg/tls/tls.go ---'
sed -n '1,220p' vendor/github.com/openshift/controller-runtime-common/pkg/tls/tls.go
echo
echo '--- cmd/local-storage-operator/main.go around the TLS logic ---'
sed -n '110,155p' cmd/local-storage-operator/main.goRepository: openshift/local-storage-operator Length of output: 8607 Check profile presence before defaulting. 🧰 Tools🪛 golangci-lint (2.12.2)[error] 125-125: : # github.com/openshift/local-storage-operator/api/v1 (typecheck) 🤖 Prompt for AI AgentsSource: MCP tools |
||
| } | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| // GetAdherencePolicyForLogging returns the current TLS adherence policy for logging purposes. | ||
| // Returns the policy and any error encountered. | ||
| func GetAdherencePolicyForLogging(ctx context.Context, c client.Client) (configv1.TLSAdherencePolicy, error) { | ||
| return crcommon.FetchAPIServerTLSAdherencePolicy(ctx, c) | ||
| } | ||
|
|
||
| // FetchAPIServerTLSProfile fetches the TLS profile spec from the cluster APIServer CR. | ||
| // This is a convenience wrapper around controller-runtime-common's function. | ||
| func FetchAPIServerTLSProfile(ctx context.Context, c client.Client) (configv1.TLSProfileSpec, error) { | ||
| return crcommon.FetchAPIServerTLSProfile(ctx, c) | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| /* | ||
| Copyright 2021 The Local Storage Operator Authors. | ||
|
|
||
| Licensed under the Apache License, Version 2.0 (the "License"); | ||
| you may not use this file except in compliance with the License. | ||
| You may obtain a copy of the License at | ||
|
|
||
| http://www.apache.org/licenses/LICENSE-2.0 | ||
|
|
||
| Unless required by applicable law or agreed to in writing, software | ||
| distributed under the License is distributed on an "AS IS" BASIS, | ||
| WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| See the License for the specific language governing permissions and | ||
| limitations under the License. | ||
| */ | ||
|
|
||
| package tls | ||
|
|
||
| import ( | ||
| "context" | ||
| "os" | ||
|
|
||
| configv1 "github.com/openshift/api/config/v1" | ||
| crcommon "github.com/openshift/controller-runtime-common/pkg/tls" | ||
| "k8s.io/klog/v2" | ||
| ) | ||
|
|
||
| // NewSecurityProfileWatcher creates a SecurityProfileWatcher that triggers operator | ||
| // restart when TLS profile or adherence policy changes. | ||
| // The initial TLS profile spec and adherence policy are passed in so the watcher | ||
| // knows the baseline configuration from operator startup. | ||
| func NewSecurityProfileWatcher( | ||
| initialProfile configv1.TLSProfileSpec, | ||
| initialAdherence configv1.TLSAdherencePolicy, | ||
| ) *crcommon.SecurityProfileWatcher { | ||
| return &crcommon.SecurityProfileWatcher{ | ||
| InitialTLSProfileSpec: initialProfile, | ||
| InitialTLSAdherencePolicy: initialAdherence, | ||
| OnProfileChange: func(ctx context.Context, oldProfile, newProfile configv1.TLSProfileSpec) { | ||
| klog.Infof("TLS profile changed, restarting operator to apply new configuration") | ||
| klog.V(2).Infof("Old profile: %+v, New profile: %+v", oldProfile, newProfile) | ||
| os.Exit(0) // Kubernetes will restart the pod | ||
| }, | ||
| OnAdherencePolicyChange: func(ctx context.Context, oldPolicy, newPolicy configv1.TLSAdherencePolicy) { | ||
| klog.Infof("TLS adherence policy changed from %s to %s, restarting operator", | ||
| oldPolicy, newPolicy) | ||
| os.Exit(0) | ||
| }, | ||
| } | ||
| } |
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.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: openshift/local-storage-operator
Length of output: 9060
🌐 Web query:
OpenShift controller-runtime-common NewTLSConfigFromProfile unsupportedCiphers omit unsupported cipher names profile contract retain supported entries rather than fail💡 Result:
In OpenShift, the controller-runtime-common utility function NewTLSConfigFromProfile is designed to handle TLS configuration from a TLSProfileSpec while gracefully managing unsupported ciphers [1][2][3]. Its contract does not fail when encountering unsupported cipher names; instead, it adopts a policy of omission, retaining only the supported entries to ensure the application maintains a functional TLS configuration [1][3]. The function returns two values: the configured TLS settings (as a function or struct) and a slice of strings containing the names of any ciphers that were unsupported or could not be mapped [4][2][3]. Standard implementation patterns across OpenShift operators include checking if this unsupported cipher slice is non-empty and logging a warning or informational message to notify administrators that specific ciphers were ignored [1][4][2][3]. This approach prioritizes system availability by applying the valid subset of the requested security profile rather than causing the operator to crash or fail to start due to minor configuration mismatches [5][3].
Citations:
🏁 Script executed:
Repository: openshift/local-storage-operator
Length of output: 1015
Don't fail
GetTLSConfigFromProfileon unsupported ciphers.crcommon.NewTLSConfigFromProfilealready drops ciphers this binary can't use and returns the supported config; returning an error here blocks metrics startup even when the profile still has usable entries. Keep the supported ciphers and log a warning for the rest.🤖 Prompt for AI Agents
Source: MCP tools