Skip to content

Commit bd45c31

Browse files
ugiordanclaude
ugiordan
and
claude
committed
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>
1 parent 104ad10 commit bd45c31

4 files changed

Lines changed: 483 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: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
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+
alpnHTTP11 = "http/1.1"
37+
)
38+
39+
type tlsBootstrapResult struct {
40+
TLSOpts []func(*tls.Config)
41+
ProfileFetched bool
42+
ProfileSpec configv1.TLSProfileSpec
43+
AdherenceFetched bool
44+
AdherencePolicy configv1.TLSAdherencePolicy
45+
UnsupportedCiphers []string
46+
}
47+
48+
func fetchTLSProfile(ctx context.Context, k8sClient client.Client) (configv1.TLSProfileSpec, bool, error) {
49+
fetchCtx, cancel := context.WithTimeout(ctx, tlsFetchTimeout)
50+
defer cancel()
51+
52+
profile, err := tlspkg.FetchAPIServerTLSProfile(fetchCtx, k8sClient)
53+
if err != nil {
54+
return classifyTLSProfileError(err)
55+
}
56+
return profile, true, nil
57+
}
58+
59+
func classifyTLSProfileError(err error) (configv1.TLSProfileSpec, bool, error) {
60+
intermediate := *configv1.TLSProfiles[configv1.TLSProfileIntermediateType]
61+
62+
switch {
63+
case apimeta.IsNoMatchError(err):
64+
return intermediate, false, nil
65+
case apierrors.IsNotFound(err):
66+
return intermediate, false, nil
67+
case isTransientError(err):
68+
return intermediate, true, nil
69+
default:
70+
return configv1.TLSProfileSpec{}, false, fmt.Errorf("unable to read APIServer TLS profile: %w", err)
71+
}
72+
}
73+
74+
func fetchTLSAdherencePolicy(ctx context.Context, k8sClient client.Client) (configv1.TLSAdherencePolicy, bool, error) {
75+
fetchCtx, cancel := context.WithTimeout(ctx, tlsFetchTimeout)
76+
defer cancel()
77+
78+
policy, err := tlspkg.FetchAPIServerTLSAdherencePolicy(fetchCtx, k8sClient)
79+
if err != nil {
80+
return configv1.TLSAdherencePolicy(""), false, nil
81+
}
82+
return policy, true, nil
83+
}
84+
85+
func bootstrapTLS(ctx context.Context, k8sClient client.Client) (*tlsBootstrapResult, error) {
86+
logger := log.FromContext(ctx)
87+
result := &tlsBootstrapResult{
88+
TLSOpts: make([]func(*tls.Config), 0, 2),
89+
}
90+
91+
profile, profileFetched, err := fetchTLSProfile(ctx, k8sClient)
92+
if err != nil {
93+
return nil, err
94+
}
95+
result.ProfileFetched = profileFetched
96+
result.ProfileSpec = profile
97+
98+
tlsConfigFn, unsupported := tlspkg.NewTLSConfigFromProfile(profile)
99+
result.UnsupportedCiphers = unsupported
100+
if len(unsupported) > 0 {
101+
logger.Info("TLS profile contains ciphers unsupported by Go", "unsupported", unsupported)
102+
}
103+
result.TLSOpts = append(result.TLSOpts, tlsConfigFn)
104+
105+
adherence, adherenceFetched, err := fetchTLSAdherencePolicy(ctx, k8sClient)
106+
if err != nil {
107+
return nil, err
108+
}
109+
result.AdherenceFetched = adherenceFetched
110+
result.AdherencePolicy = adherence
111+
112+
result.TLSOpts = append(result.TLSOpts, func(c *tls.Config) {
113+
c.NextProtos = []string{"h2", alpnHTTP11}
114+
})
115+
116+
return result, nil
117+
}
118+
119+
func isTransientError(err error) bool {
120+
return apierrors.IsServiceUnavailable(err) ||
121+
apierrors.IsTimeout(err) ||
122+
apierrors.IsServerTimeout(err) ||
123+
apierrors.IsTooManyRequests(err) ||
124+
errors.Is(err, context.DeadlineExceeded)
125+
}

0 commit comments

Comments
 (0)