feat(autoscaler)!: Remove caBundle requirement for HTTPS URLs#4332
Merged
lacroixthomas merged 2 commits intoagones-dev:mainfrom Nov 19, 2025
Merged
feat(autoscaler)!: Remove caBundle requirement for HTTPS URLs#4332lacroixthomas merged 2 commits intoagones-dev:mainfrom
lacroixthomas merged 2 commits intoagones-dev:mainfrom
Conversation
BREAKING CHANGE: HTTPS webhook and WASM URLs no longer require caBundle when configured. The impact of this is that HTTPS URLs for a webhook or wasm package that are served via a trusted certificate authority will be processed successfully. This is particularly important for Wasm based autoscaling, as WASM packages will likely be served from Cloud storage providers, via HTTPS which is not controlled by the end user. The caBundle field is now only required for URLs using custom or self-signed certificates, as they will fail when being called from the Fleet Autoscaler. This simplifies configuration for standard HTTPS endpoints while maintaining security for custom certificate scenarios. Changes: - Updated URL validation logic to allow HTTPS without caBundle - Modified HTTP client to be stored per Fleet Autoscaler, allowing for a custom http client per Fleet Autoscaler. - Updated tests to verify new behavior - Adjusted validation error messages
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a breaking change to remove the requirement for caBundle when using HTTPS URLs in Fleet Autoscaler webhook and WASM configurations. It allows HTTPS URLs to use system-trusted certificate authorities while maintaining the option to provide custom certificates via caBundle for self-signed or custom certificates.
- Removed validation requiring
caBundlefor HTTPS URLs - Modified HTTP client to be stored per Fleet Autoscaler instance with appropriate TLS configuration
- Updated
buildURLFromConfigurationto handle both trusted and custom certificate scenarios
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pkg/apis/autoscaling/v1/fleetautoscaler.go | Removed validation that required CABundle for HTTPS URLs |
| pkg/apis/autoscaling/v1/fleetautoscaler_test.go | Updated test to expect no validation errors for HTTPS without caBundle |
| pkg/fleetautoscalers/fleetautoscalers.go | Refactored to store HTTP client per Fleet Autoscaler, moved TLS configuration setup, and updated function signatures |
| pkg/fleetautoscalers/fleetautoscalers_test.go | Added comprehensive tests for URL configuration building with various scenarios |
| pkg/fleetautoscalers/controller.go | Added httpClient field to fasState and cleanup logic for idle connections |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
220
to
223
| scheme := "http" | ||
| if w.CABundle != nil { | ||
| scheme = "https" | ||
|
|
||
| if err := setCABundle(w.CABundle); err != nil { | ||
| return nil, err | ||
| } | ||
| } |
There was a problem hiding this comment.
The scheme variable is only used for Service configurations (line 250) but not for URL configurations. When a URL is provided (lines 225-231), it's parsed directly using url.ParseRequestURI(*w.URL), which preserves the URL's original scheme. This makes lines 220-223 misleading, as they suggest the scheme depends on CABundle presence, but this only applies to Service configurations, not URLs.
Consider moving the scheme determination to the Service configuration block (after line 232) to make it clear it only applies to Service configurations, or add a comment explaining this distinction.
Collaborator
|
Build Succeeded 🥳 Build Id: 0826182b-ff14-4034-af99-5be46ecc6c44 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version: |
Collaborator
|
Looks all good ! 👌🏼 |
lacroixthomas
approved these changes
Nov 19, 2025
Collaborator
|
Build Succeeded 🥳 Build Id: 168edc25-6249-4c31-8f24-8fe3357be382 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version: |
mnthe
pushed a commit
to mnthe/agones
that referenced
this pull request
Mar 23, 2026
…-dev#4332) BREAKING CHANGE: HTTPS webhook and WASM URLs no longer require caBundle when configured. The impact of this is that HTTPS URLs for a webhook or wasm package that are served via a trusted certificate authority will be processed successfully. This is particularly important for Wasm based autoscaling, as WASM packages will likely be served from Cloud storage providers, via HTTPS which is not controlled by the end user. The caBundle field is now only required for URLs using custom or self-signed certificates, as they will fail when being called from the Fleet Autoscaler. This simplifies configuration for standard HTTPS endpoints while maintaining security for custom certificate scenarios. Changes: - Updated URL validation logic to allow HTTPS without caBundle - Modified HTTP client to be stored per Fleet Autoscaler, allowing for a custom http client per Fleet Autoscaler. - Updated tests to verify new behavior - Adjusted validation error messages Co-authored-by: Thomas Lacroix <thomas.lacroix@ubisoft.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What type of PR is this?
/kind feature
What this PR does / Why we need it:
BREAKING CHANGE: HTTPS webhook and WASM URLs no longer require caBundle when configured. The impact of this is that HTTPS URLs for a webhook or wasm package that are served via a trusted certificate authority will be processed successfully. This is particularly important for Wasm based autoscaling, as WASM packages will likely be served from Cloud storage providers, via HTTPS which is not controlled by the end user.
The caBundle field is now only required for URLs using custom or self-signed certificates, as they will fail when being called from the Fleet Autoscaler.
This simplifies configuration for standard HTTPS endpoints while maintaining security for custom certificate scenarios.
Changes:
Which issue(s) this PR fixes:
Work on #4080
Special notes for your reviewer:
This will also make things easier if people want to host webhooks on a functions as a service, or serverless platform for ease of use -- since they often won't control the certificates in those environments.