Skip to content

Commit 43ae993

Browse files
ugiordanclaude
ugiordan
and
claude
authored
feat: Apply Intermediate TLS defaults on API fallback and handle transient errors (feast-dev#6587)
* fix: handle transient API errors and improve TLS profile fallback Two fixes to the TLS profile integration: 1. NewTLSConfigFromProfile was only called in the success branch of the TLS profile fetch. On error paths (non-OpenShift, not found), no TLS config was applied, leaving Go's bare defaults. Now it always runs with an explicit Intermediate fallback on all error paths. 2. Transient API errors (ServiceUnavailable, Timeout, ServerTimeout, TooManyRequests, DeadlineExceeded) crashed the operator. Now they fall back to Intermediate defaults and set ProfileFetched=true so the SecurityProfileWatcher self-heals when the API recovers. The TLS bootstrap logic is extracted into tls_bootstrap.go with comprehensive unit tests covering all error classification edge cases: transient vs fatal vs graceful fallback, Intermediate defaults always applied, and ALPN configuration. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Ugo Giordano <ugiordan@redhat.com> * fix: classify TLS adherence errors instead of swallowing them fetchTLSAdherencePolicy was silently returning nil error on all error paths, making the error check in bootstrapTLS dead code. Apply the same error classification pattern as fetchTLSProfile (NoMatch/NotFound are non-OpenShift fallback, transient errors retry, unexpected errors crash). Also add alpnH2 constant for consistency with alpnHTTP11. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Ugo Giordano <ugiordan@redhat.com> --------- Signed-off-by: Ugo Giordano <ugiordan@redhat.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 75b9463 commit 43ae993

4 files changed

Lines changed: 492 additions & 47 deletions

File tree

‎infra/feast-operator/Dockerfile‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ COPY --chown=1001:0 go.sum go.sum
1212
RUN go mod download
1313

1414
# Copy the go source
15-
COPY --chown=1001:0 cmd/main.go cmd/main.go
15+
COPY --chown=1001:0 cmd/ cmd/
1616
COPY --chown=1001:0 api/ api/
1717
COPY --chown=1001:0 internal/controller/ internal/controller/
1818

@@ -21,7 +21,7 @@ COPY --chown=1001:0 internal/controller/ internal/controller/
2121
# was called. For example, if we call make docker-build in a local env which has the Apple Silicon M1 SO
2222
# the docker BUILDPLATFORM arg will be linux/arm64 when for Apple x86 it will be linux/amd64. Therefore,
2323
# by leaving it empty we can ensure that the container and binary shipped on it will have the same platform.
24-
RUN CGO_ENABLED=0 GOOS=${TARGETOS:-linux} GOARCH=${TARGETARCH} go build -a -o manager cmd/main.go
24+
RUN CGO_ENABLED=0 GOOS=${TARGETOS:-linux} GOARCH=${TARGETARCH} go build -a -o manager ./cmd/
2525

2626
FROM registry.access.redhat.com/ubi9/ubi-minimal:9.8
2727
WORKDIR /

‎infra/feast-operator/cmd/main.go‎

Lines changed: 9 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,6 @@ import (
3434
corev1 "k8s.io/api/core/v1"
3535
policyv1 "k8s.io/api/policy/v1"
3636
rbacv1 "k8s.io/api/rbac/v1"
37-
apierrors "k8s.io/apimachinery/pkg/api/errors"
38-
apimeta "k8s.io/apimachinery/pkg/api/meta"
3937
"k8s.io/apimachinery/pkg/labels"
4038
"k8s.io/apimachinery/pkg/runtime"
4139
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
@@ -102,7 +100,7 @@ func main() {
102100
var probeAddr string
103101
var secureMetrics bool
104102
var featureStoreMetrics bool
105-
var tlsOpts []func(*tls.Config)
103+
tlsOpts := make([]func(*tls.Config), 0, 2)
106104
flag.StringVar(&metricsAddr, "metrics-bind-address", "0", "The address the metrics endpoint binds to. "+
107105
"Use :8443 for HTTPS or :8080 for HTTP, or leave as 0 to disable the metrics service.")
108106
flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.")
@@ -130,46 +128,12 @@ func main() {
130128
os.Exit(1)
131129
}
132130

133-
tlsProfileFetched := false
134-
tlsProfile, err := tlspkg.FetchAPIServerTLSProfile(context.Background(), bootstrapClient)
131+
tlsResult, err := bootstrapTLS(context.Background(), bootstrapClient)
135132
if err != nil {
136-
switch {
137-
case apimeta.IsNoMatchError(err):
138-
setupLog.Info("TLS profile not available, using hardened defaults (non-OpenShift cluster)")
139-
case apierrors.IsNotFound(err):
140-
setupLog.Info("APIServer resource not found, using hardened defaults")
141-
default:
142-
setupLog.Error(err, "unable to read APIServer TLS profile, refusing to start with unknown TLS posture")
143-
os.Exit(1)
144-
}
145-
} else {
146-
tlsProfileFetched = true
147-
tlsConfigFn, unsupported := tlspkg.NewTLSConfigFromProfile(tlsProfile)
148-
if len(unsupported) > 0 {
149-
setupLog.Info("TLS profile contains ciphers unsupported by Go", "unsupported", unsupported)
150-
}
151-
tlsOpts = append(tlsOpts, tlsConfigFn)
152-
}
153-
154-
tlsAdherenceFetched := false
155-
tlsAdherence, err := tlspkg.FetchAPIServerTLSAdherencePolicy(context.Background(), bootstrapClient)
156-
if err != nil {
157-
switch {
158-
case apimeta.IsNoMatchError(err):
159-
setupLog.Info("TLS adherence policy not available (non-OpenShift cluster)")
160-
case apierrors.IsNotFound(err):
161-
setupLog.Info("APIServer resource not found, skipping adherence policy")
162-
default:
163-
setupLog.Error(err, "unable to read APIServer TLS adherence policy, refusing to start")
164-
os.Exit(1)
165-
}
166-
} else {
167-
tlsAdherenceFetched = true
133+
setupLog.Error(err, "TLS bootstrap failed")
134+
os.Exit(1)
168135
}
169-
170-
tlsOpts = append(tlsOpts, func(c *tls.Config) {
171-
c.NextProtos = []string{"h2", "http/1.1"}
172-
})
136+
tlsOpts = append(tlsOpts, tlsResult.TLSOpts...)
173137

174138
webhookServer := webhook.NewServer(webhook.Options{
175139
TLSOpts: tlsOpts,
@@ -271,17 +235,17 @@ func main() {
271235
ctx, cancel := context.WithCancel(ctrl.SetupSignalHandler())
272236
defer cancel()
273237

274-
if tlsProfileFetched {
238+
if tlsResult.ProfileFetched {
275239
watcher := &tlspkg.SecurityProfileWatcher{
276240
Client: mgr.GetClient(),
277-
InitialTLSProfileSpec: tlsProfile,
241+
InitialTLSProfileSpec: tlsResult.ProfileSpec,
278242
OnProfileChange: func(_ context.Context, _, _ configv1.TLSProfileSpec) {
279243
setupLog.Info("TLS profile changed, initiating shutdown to reload")
280244
cancel()
281245
},
282246
}
283-
if tlsAdherenceFetched {
284-
watcher.InitialTLSAdherencePolicy = tlsAdherence
247+
if tlsResult.AdherenceFetched {
248+
watcher.InitialTLSAdherencePolicy = tlsResult.AdherencePolicy
285249
watcher.OnAdherencePolicyChange = func(_ context.Context, _, _ configv1.TLSAdherencePolicy) {
286250
setupLog.Info("TLS adherence policy changed, initiating shutdown to reload")
287251
cancel()

‎infra/feast-operator/cmd/tls_bootstrap.go‎

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
/*
2+
Copyright 2024 Feast Community.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package main
18+
19+
import (
20+
"context"
21+
"crypto/tls"
22+
"errors"
23+
"fmt"
24+
"time"
25+
26+
configv1 "github.com/openshift/api/config/v1"
27+
tlspkg "github.com/openshift/controller-runtime-common/pkg/tls"
28+
apierrors "k8s.io/apimachinery/pkg/api/errors"
29+
apimeta "k8s.io/apimachinery/pkg/api/meta"
30+
"sigs.k8s.io/controller-runtime/pkg/client"
31+
"sigs.k8s.io/controller-runtime/pkg/log"
32+
)
33+
34+
const (
35+
tlsFetchTimeout = 10 * time.Second
36+
alpnH2 = "h2"
37+
alpnHTTP11 = "http/1.1"
38+
)
39+
40+
type tlsBootstrapResult struct {
41+
TLSOpts []func(*tls.Config)
42+
ProfileFetched bool
43+
ProfileSpec configv1.TLSProfileSpec
44+
AdherenceFetched bool
45+
AdherencePolicy configv1.TLSAdherencePolicy
46+
UnsupportedCiphers []string
47+
}
48+
49+
func fetchTLSProfile(ctx context.Context, k8sClient client.Client) (configv1.TLSProfileSpec, bool, error) {
50+
fetchCtx, cancel := context.WithTimeout(ctx, tlsFetchTimeout)
51+
defer cancel()
52+
53+
profile, err := tlspkg.FetchAPIServerTLSProfile(fetchCtx, k8sClient)
54+
if err != nil {
55+
return classifyTLSProfileError(err)
56+
}
57+
return profile, true, nil
58+
}
59+
60+
func classifyTLSProfileError(err error) (configv1.TLSProfileSpec, bool, error) {
61+
intermediate := *configv1.TLSProfiles[configv1.TLSProfileIntermediateType]
62+
63+
switch {
64+
case apimeta.IsNoMatchError(err):
65+
return intermediate, false, nil
66+
case apierrors.IsNotFound(err):
67+
return intermediate, false, nil
68+
case isTransientError(err):
69+
return intermediate, true, nil
70+
default:
71+
return configv1.TLSProfileSpec{}, false, fmt.Errorf("unable to read APIServer TLS profile: %w", err)
72+
}
73+
}
74+
75+
func fetchTLSAdherencePolicy(ctx context.Context, k8sClient client.Client) (configv1.TLSAdherencePolicy, bool, error) {
76+
fetchCtx, cancel := context.WithTimeout(ctx, tlsFetchTimeout)
77+
defer cancel()
78+
79+
policy, err := tlspkg.FetchAPIServerTLSAdherencePolicy(fetchCtx, k8sClient)
80+
if err == nil {
81+
return policy, true, nil
82+
}
83+
84+
switch {
85+
case apimeta.IsNoMatchError(err),
86+
apierrors.IsNotFound(err),
87+
isTransientError(err):
88+
return "", false, nil
89+
default:
90+
return "", false, fmt.Errorf("unable to read APIServer TLS adherence policy: %w", err)
91+
}
92+
}
93+
94+
func bootstrapTLS(ctx context.Context, k8sClient client.Client) (*tlsBootstrapResult, error) {
95+
logger := log.FromContext(ctx)
96+
result := &tlsBootstrapResult{
97+
TLSOpts: make([]func(*tls.Config), 0, 2),
98+
}
99+
100+
profile, profileFetched, err := fetchTLSProfile(ctx, k8sClient)
101+
if err != nil {
102+
return nil, err
103+
}
104+
result.ProfileFetched = profileFetched
105+
result.ProfileSpec = profile
106+
107+
tlsConfigFn, unsupported := tlspkg.NewTLSConfigFromProfile(profile)
108+
result.UnsupportedCiphers = unsupported
109+
if len(unsupported) > 0 {
110+
logger.Info("TLS profile contains ciphers unsupported by Go", "unsupported", unsupported)
111+
}
112+
result.TLSOpts = append(result.TLSOpts, tlsConfigFn)
113+
114+
adherence, adherenceFetched, err := fetchTLSAdherencePolicy(ctx, k8sClient)
115+
if err != nil {
116+
return nil, err
117+
}
118+
result.AdherenceFetched = adherenceFetched
119+
result.AdherencePolicy = adherence
120+
121+
result.TLSOpts = append(result.TLSOpts, func(c *tls.Config) {
122+
c.NextProtos = []string{alpnH2, alpnHTTP11}
123+
})
124+
125+
return result, nil
126+
}
127+
128+
func isTransientError(err error) bool {
129+
return apierrors.IsServiceUnavailable(err) ||
130+
apierrors.IsTimeout(err) ||
131+
apierrors.IsServerTimeout(err) ||
132+
apierrors.IsTooManyRequests(err) ||
133+
errors.Is(err, context.DeadlineExceeded)
134+
}

0 commit comments

Comments
 (0)