Skip to content

feat: Apply Intermediate TLS defaults on API fallback and handle transient errors#6587

Open
ugiordan wants to merge 419 commits into
feast-dev:masterfrom
ugiordan:feat/tls-intermediate-fallback
Open

feat: Apply Intermediate TLS defaults on API fallback and handle transient errors#6587
ugiordan wants to merge 419 commits into
feast-dev:masterfrom
ugiordan:feat/tls-intermediate-fallback

Conversation

@ugiordan

@ugiordan ugiordan commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

What this PR does / why we need it:

Follow-up to #6567. Fixes two issues in the TLS profile integration:

  1. Else-only pattern: NewTLSConfigFromProfile was only called in the success branch of the TLS profile fetch. On any error path (non-OpenShift cluster, resource not found), no TLS config was applied, leaving the operator running with Go's bare defaults (no MinVersion, no cipher restrictions). This PR moves NewTLSConfigFromProfile outside the if/else and explicitly falls back to configv1.TLSProfiles[TLSProfileIntermediateType] on all error paths.

  2. No transient error handling: The error switch only handled IsNotFound and IsNoMatchError. Transient API errors (ServiceUnavailable, Timeout, TooManyRequests) hit the default case and crashed the operator with os.Exit(1). This PR adds a dedicated case for these errors with a graceful Intermediate fallback. tlsProfileFetched is set to true so the SecurityProfileWatcher self-heals when the API recovers.

Additionally:

  • Adds a 10-second context timeout on bootstrap API calls (previously used context.Background() with no deadline).
  • Applies the same transient error handling to the TLS adherence policy fetch.

Which issue(s) this PR fixes:

Follow-up to #6567.

Checks

  • I've made sure the tests are passing.
  • My commits are signed off (git commit -s)
  • My PR title follows conventional commits format

Testing Strategy

  • Manual tests
  • Testing is not required for this change

Misc

The operator Go module builds cleanly with go build ./... from infra/feast-operator/.

m-rafeeq and others added 30 commits December 30, 2025 16:08
Signed-off-by: Srihari <svenkata@redhat.com>
Signed-off-by: Srihari <svenkata@redhat.com>
@ugiordan
ugiordan requested a review from a team as a code owner July 7, 2026 15:55
@ugiordan
ugiordan force-pushed the feat/tls-intermediate-fallback branch from c9041d5 to e3c4048 Compare July 7, 2026 16:03
@ugiordan
ugiordan force-pushed the feat/tls-intermediate-fallback branch 3 times, most recently from 846c5a6 to 60296ac Compare July 7, 2026 16:32
@ugiordan
ugiordan force-pushed the feat/tls-intermediate-fallback branch 2 times, most recently from 1375b6f to 748ecc5 Compare July 9, 2026 08:38
@ntkathole
ntkathole force-pushed the feat/tls-intermediate-fallback branch from 748ecc5 to fe29daa Compare July 14, 2026 06:01
@codecov-commenter

codecov-commenter commented Jul 16, 2026

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 90.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 46.01%. Comparing base (14e0a83) to head (c7b9f67).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
sdk/python/feast/metrics.py 0.00% 1 Missing ⚠️
.../feast/permissions/auth/kubernetes_token_parser.py 94.73% 1 Missing ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6587      +/-   ##
==========================================
- Coverage   46.02%   46.01%   -0.02%     
==========================================
  Files         402      406       +4     
  Lines       47656    48175     +519     
  Branches     6741     6816      +75     
==========================================
+ Hits        21936    22166     +230     
- Misses      24196    24463     +267     
- Partials     1524     1546      +22     
Flag Coverage Δ
go-feature-server 30.58% <ø> (ø)
python-unit 47.33% <90.00%> (-0.04%) ⬇️
Files with missing lines Coverage Δ
sdk/python/feast/metrics.py 74.47% <0.00%> (-0.53%) ⬇️
.../feast/permissions/auth/kubernetes_token_parser.py 41.00% <94.73%> (+2.90%) ⬆️

... and 7 files with indirect coverage changes


Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 14e0a83...c7b9f67. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

ugiordan and others added 2 commits July 16, 2026 17:03
…sient errors

Follow-up to feast-dev#6567. Two fixes:

1. Move NewTLSConfigFromProfile outside the if/else so it runs on all
   code paths. Previously, error paths skipped TLS configuration entirely,
   leaving the operator running with Go's bare defaults (no MinVersion,
   no cipher restrictions). Now all error paths explicitly fall back to
   the Intermediate TLS profile.

2. Handle transient API errors (ServiceUnavailable, Timeout,
   TooManyRequests) gracefully instead of crashing with os.Exit(1).
   These set tlsProfileFetched=true so the SecurityProfileWatcher
   self-heals when the API recovers.

Also adds a 10-second context timeout on bootstrap API calls and applies
the same transient error handling to the TLS adherence policy fetch.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Ugo Giordano <ugiordan@redhat.com>
When context.WithTimeout fires due to a slow API server, the resulting
context.DeadlineExceeded error does not match any apierrors.Is* check
and falls into the fatal default case with os.Exit(1), crashing the
operator. Add it to the transient error branch so the operator
gracefully falls back to Intermediate TLS defaults instead.

Signed-off-by: Ugo Giordano <ugiordan@redhat.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Ugo Giordano <ugiordan@redhat.com>
@ugiordan
ugiordan force-pushed the feat/tls-intermediate-fallback branch from 4591db8 to da15060 Compare July 16, 2026 15:03
StatusReasonServerTimeout is a distinct Kubernetes error from
StatusReasonTimeout. Without handling it, a server-side API timeout
during TLS profile fetch causes the operator to fail to start instead
of falling back to Intermediate defaults.

Signed-off-by: Ugo Giordano <ugiordan@redhat.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@ugiordan
ugiordan force-pushed the feat/tls-intermediate-fallback branch from da15060 to c7b9f67 Compare July 17, 2026 13:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.