Skip to content

fix: use https_verify_certificate for standalone active health checks #396

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.

Sign up for GitHub

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

Merged
AlinsRan merged 3 commits into from
Apr 30, 2026
+17 −11
Merged

fix: use https_verify_certificate for standalone active health checks #396

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f507224
fix: align standalone health check verify field with ADC
AlinsRan Apr 30, 2026
4fa0906
chore: drop standalone health check translator test
AlinsRan Apr 30, 2026
7250f04
ci: enable dev ADC in API7EE e2e workflows
AlinsRan Apr 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .github/workflows/e2e-test-k8s.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ concurrency:
group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}
cancel-in-progress: true

env:
ADC_VERSION: dev

Comment on lines +31 to +33
jobs:
e2e-test:
strategy:
Expand Down
3 changes: 3 additions & 0 deletions .github/workflows/e2e-test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ concurrency:
group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}
cancel-in-progress: true

env:
ADC_VERSION: dev

Comment on lines +31 to +33
jobs:
e2e-test:
strategy:
Expand Down
20 changes: 10 additions & 10 deletions api/adc/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,16 +198,16 @@ type ClientTLS struct {
// UpstreamActiveHealthCheck defines the active upstream health check configuration.
// +k8s:deepcopy-gen=true
type UpstreamActiveHealthCheck struct {
Type string `json:"type,omitempty" yaml:"type,omitempty"`
Timeout int `json:"timeout,omitempty" yaml:"timeout,omitempty"`
Concurrency int `json:"concurrency,omitempty" yaml:"concurrency,omitempty"`
Host string `json:"host,omitempty" yaml:"host,omitempty"`
Port int32 `json:"port,omitempty" yaml:"port,omitempty"`
HTTPPath string `json:"http_path,omitempty" yaml:"http_path,omitempty"`
HTTPSVerifyCert bool `json:"https_verify_cert,omitempty" yaml:"https_verify_cert,omitempty"`
HTTPRequestHeaders []string `json:"req_headers,omitempty" yaml:"req_headers,omitempty"`
Healthy UpstreamActiveHealthCheckHealthy `json:"healthy,omitempty" yaml:"healthy,omitempty"`
Unhealthy UpstreamActiveHealthCheckUnhealthy `json:"unhealthy,omitempty" yaml:"unhealthy,omitempty"`
Type string `json:"type,omitempty" yaml:"type,omitempty"`
Timeout int `json:"timeout,omitempty" yaml:"timeout,omitempty"`
Concurrency int `json:"concurrency,omitempty" yaml:"concurrency,omitempty"`
Host string `json:"host,omitempty" yaml:"host,omitempty"`
Port int32 `json:"port,omitempty" yaml:"port,omitempty"`
HTTPPath string `json:"http_path,omitempty" yaml:"http_path,omitempty"`
HTTPSVerifyCertificate bool `json:"https_verify_certificate,omitempty" yaml:"https_verify_certificate,omitempty"`
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🌐 Web query:

In Go's encoding/json and common YAML marshalling, does omitemptyomit aboolfield when its value isfalse? Also, for APISIX upstream active health checks, what is the default behavior of https_verify_certificate when the field is omitted?

💡 Result:

  1. Go encoding/json + common YAML marshalling: does omitempty omit a bool when it is false? Yes for Go’s encoding/json. In Go’s encoding/json, a struct field tagged with omitempty is omitted when the field has an “empty value”; for bool, that empty value is false. 2) For APISIX upstream active health checks: default behavior of https_verify_certificate when omitted When using APISIX active health checks with HTTPS probes, the field upstream.checks.active.https_verify_certificate defaults to true (i.e., certificate verification is performed).

Citations:

Fix the TLS verification field to prevent silent inversion of boolean state.

The HTTPSVerifyCertificate field is a plain bool with omitempty. When StrictTLS: false (intending to disable certificate verification), the field is set to false, which then gets omitted from the serialized output due to omitempty. APISIX defaults the missing https_verify_certificate field to true, inverting the intended behavior—users disabling verification end up with verification enabled instead.

Change this to *bool to preserve the explicit false state, or remove the omitempty tag to always emit the field. This is a security-relevant configuration issue.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@api/adc/types.go` at line 207, The HTTPSVerifyCertificate field in the ADC
config struct is currently a plain bool with `omitempty`, which causes explicit
false to be dropped and APISIX to default to true; change the field declaration
for HTTPSVerifyCertificate to be a pointer (*bool) so false is preserved (or
alternatively remove `omitempty` from its `json`/`yaml` tags) and update any
constructors or JSON/YAML marshalling assumptions that set or read
HTTPSVerifyCertificate to handle a nil vs. non-nil value (look for the
struct/type that declares HTTPSVerifyCertificate and any code paths that build
or serialize that struct).

HTTPRequestHeaders []string `json:"req_headers,omitempty" yaml:"req_headers,omitempty"`
Healthy UpstreamActiveHealthCheckHealthy `json:"healthy,omitempty" yaml:"healthy,omitempty"`
Unhealthy UpstreamActiveHealthCheckUnhealthy `json:"unhealthy,omitempty" yaml:"unhealthy,omitempty"`
}

// UpstreamPassiveHealthCheck defines the passive health check configuration for an upstream.
Expand Down
2 changes: 1 addition & 1 deletion internal/adc/translator/apisixupstream.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ func translateUpstreamActiveHealthCheck(config *apiv2.ActiveHealthCheck) (*adc.U
active.HTTPRequestHeaders = config.RequestHeaders

if config.StrictTLS == nil || *config.StrictTLS {
active.HTTPSVerifyCert = true
active.HTTPSVerifyCertificate = true
}
Comment on lines 323 to 327

if config.Healthy != nil {
Expand Down
Loading