Skip to content
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.

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

check: Add support for Consul field tls_server_name #17334

Merged
merged 5 commits into from
Jun 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 .changelog/17334.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
checks: Added support for Consul check field tls_server_name
```
1 change: 1 addition & 0 deletions api/services.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,7 @@ type ServiceCheck struct {
Interval time.Duration `hcl:"interval,optional"`
Timeout time.Duration `hcl:"timeout,optional"`
InitialStatus string `mapstructure:"initial_status" hcl:"initial_status,optional"`
TLSServerName string `mapstructure:"tls_server_name" hcl:"tls_server_name,optional"`
TLSSkipVerify bool `mapstructure:"tls_skip_verify" hcl:"tls_skip_verify,optional"`
Header map[string][]string `hcl:"header,block"`
Method string `hcl:"method,optional"`
Expand Down
3 changes: 3 additions & 0 deletions command/agent/consul/service_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -1164,6 +1164,7 @@ func apiCheckRegistrationToCheck(r *api.AgentCheckRegistration) *api.AgentServic
Body: r.Body,
TCP: r.TCP,
Status: r.Status,
TLSServerName: r.TLSServerName,
TLSSkipVerify: r.TLSSkipVerify,
GRPC: r.GRPC,
GRPCUseTLS: r.GRPCUseTLS,
Expand Down Expand Up @@ -1653,6 +1654,7 @@ func createCheckReg(serviceID, checkID string, check *structs.ServiceCheck, host
if check.TLSSkipVerify {
chkReg.TLSSkipVerify = true
}
chkReg.TLSServerName = check.TLSServerName
base := url.URL{
Scheme: proto,
Host: net.JoinHostPort(host, strconv.Itoa(port)),
Expand Down Expand Up @@ -1681,6 +1683,7 @@ func createCheckReg(serviceID, checkID string, check *structs.ServiceCheck, host
if check.TLSSkipVerify {
chkReg.TLSSkipVerify = true
}
chkReg.TLSServerName = check.TLSServerName

default:
return nil, fmt.Errorf("check type %+q not valid", check.Type)
Expand Down
6 changes: 4 additions & 2 deletions command/agent/consul/unit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -972,6 +972,7 @@ func TestCreateCheckReg_GRPC(t *testing.T) {
PortLabel: "label",
GRPCService: "foo.Bar",
GRPCUseTLS: true,
TLSServerName: "localhost",
TLSSkipVerify: true,
Timeout: time.Second,
Interval: time.Minute,
Expand All @@ -988,13 +989,14 @@ func TestCreateCheckReg_GRPC(t *testing.T) {
AgentServiceCheck: api.AgentServiceCheck{
Timeout: "1s",
Interval: "1m0s",
GRPC: "localhost:8080/foo.Bar",
GRPC: "127.0.0.1:8080/foo.Bar",
GRPCUseTLS: true,
TLSServerName: "localhost",
TLSSkipVerify: true,
},
}

actual, err := createCheckReg(serviceID, checkID, check, "localhost", 8080, "default")
actual, err := createCheckReg(serviceID, checkID, check, "127.0.0.1", 8080, "default")
must.NoError(t, err)
must.Eq(t, expected, actual)
}
Expand Down
1 change: 1 addition & 0 deletions command/agent/job_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -1482,6 +1482,7 @@ func ApiServicesToStructs(in []*api.Service, group bool) []*structs.Service {
Interval: check.Interval,
Timeout: check.Timeout,
InitialStatus: check.InitialStatus,
TLSServerName: check.TLSServerName,
TLSSkipVerify: check.TLSSkipVerify,
Header: check.Header,
Method: check.Method,
Expand Down
12 changes: 12 additions & 0 deletions nomad/structs/diff_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3015,6 +3015,12 @@ func TestTaskGroupDiff(t *testing.T) {
Old: "3",
New: "5",
},
{
Type: DiffTypeNone,
Name: "TLSServerName",
Old: "",
New: "",
},
{
Type: DiffTypeNone,
Name: "TLSSkipVerify",
Expand Down Expand Up @@ -6555,6 +6561,12 @@ func TestTaskDiff(t *testing.T) {
Old: "4",
New: "4",
},
{
Type: DiffTypeNone,
Name: "TLSServerName",
Old: "",
New: "",
},
{
Type: DiffTypeNone,
Name: "TLSSkipVerify",
Expand Down
15 changes: 14 additions & 1 deletion nomad/structs/services.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,8 @@ type ServiceCheck struct {
Interval time.Duration // Interval of the check
Timeout time.Duration // Timeout of the response from the check before consul fails the check
InitialStatus string // Initial status of the check
TLSSkipVerify bool // Skip TLS verification when Protocol=https
TLSServerName string // ServerName to use for SNI and TLS verification when (Type=https and Protocol=https) or (Type=grpc and GRPCUseTLS=true)
tgross marked this conversation as resolved.
Show resolved Hide resolved
TLSSkipVerify bool // Skip TLS verification when (type=https and Protocol=https) or (type=grpc and grpc_use_tls=true)
Method string // HTTP Method to use (GET by default)
Header map[string][]string // HTTP Headers for Consul to set when making HTTP checks
CheckRestart *CheckRestart // If and when a task should be restarted based on checks
Expand Down Expand Up @@ -183,6 +184,10 @@ func (sc *ServiceCheck) Equal(o *ServiceCheck) bool {
return false
}

if sc.TLSServerName != o.TLSServerName {
return false
}

if sc.Timeout != o.Timeout {
return false
}
Expand Down Expand Up @@ -378,6 +383,11 @@ func (sc *ServiceCheck) validateNomad() error {
return fmt.Errorf("failures_before_critical may only be set for Consul service checks")
}

// tls_server_name is consul only
if sc.TLSServerName != "" {
return fmt.Errorf("tls_server_name may only be set for Consul service checks")
}

return nil
}

Expand Down Expand Up @@ -465,6 +475,9 @@ func (sc *ServiceCheck) Hash(serviceID string) string {
// use name "true" to maintain ID stability
hashBool(h, sc.TLSSkipVerify, "true")

// Only include TLSServerName if set to maintain ID stability with Nomad <1.6.0
hashStringIfNonEmpty(h, sc.TLSServerName)

// maintain artisanal map hashing to maintain ID stability
hashHeader(h, sc.Header)

Expand Down
11 changes: 11 additions & 0 deletions nomad/structs/services_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,17 @@ func TestServiceCheck_validateNomad(t *testing.T) {
Body: "this is a request payload!",
},
},
{
name: "http with tls_server_name",
sc: &ServiceCheck{
Type: ServiceCheckHTTP,
Interval: 3 * time.Second,
Timeout: 1 * time.Second,
Path: "/health",
TLSServerName: "foo",
},
exp: `tls_server_name may only be set for Consul service checks`,
},
}

for _, testCase := range testCases {
Expand Down
7 changes: 7 additions & 0 deletions ui/stories/components/diff-viewer.stories.js
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,13 @@ export let DiffViewerWithManyChanges = () => {
Old: '',
Type: 'None',
},
{
Annotations: null,
Name: 'TLSServerName',
New: '',
Old: '',
Type: 'None',
},
{
Annotations: null,
Name: 'TLSSkipVerify',
Expand Down
25 changes: 23 additions & 2 deletions website/content/docs/job-specification/check.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ job "example" {

- `grpc_use_tls` `(bool: false)` - Use TLS to perform a gRPC health check. May
be used with `tls_skip_verify` to use TLS but skip certificate verification.
May be used with `tls_server_name` to specify the ServerName to use for SNI
and validation of the certificate presented by the server being checked.

- `initial_status` `(string: <enum>)` - Specifies the starting status of the
service. Valid options are `passing`, `warning`, and `critical`. Omitting
Expand Down Expand Up @@ -157,8 +159,27 @@ job "example" {
Nomad. For Consul service checks, valid options are `grpc`, `http`, `script`,
and `tcp`. For Nomad service checks, valid options are `http` and `tcp`.

- `tls_skip_verify` `(bool: false)` - Skip verifying TLS certificates for HTTPS
checks. Only supported in the Consul service provider.
- `tls_server_name` `(string: "")` - Indicates the ServerName to use for SNI and
validation of the certificate presented by the server being checked, when
performing TLS enabled checks (`https` and `grpc` with `grpc_use_tls`). If left
unspecified, the ServerName will be inferred from the address of the server
being checked unless the address is an IP address. There are two common cases
where this is beneficial:

- When the check address contains an IP, `tls_server_name` can be specified
for SNI. Note: setting `tls_server_name` will also override the hostname
used to verify the certificate presented by the server being checked.

- When the check address contains a hostname which isn't be present in the
SAN (Subject Alternative Name) field of the certificate presented by the
server being checked. Note: setting `tls_server_name` will also override
the hostname used for SNI.

This field is only supported in the Consul service provider.

- `tls_skip_verify` `(bool: false)` - Skip verification of certificates for
`https` and `grpc` with `grpc_use_tls` checks . Only supported in the Consul
service provider.

- `on_update` `(string: "require_healthy")` - Specifies how checks should be
evaluated when determining deployment health (including a job's initial
Expand Down