Skip to content

Commit

Permalink
config: remove old Vault/Consul config blocks from server (#18991)
Browse files Browse the repository at this point in the history
Remove the now-unused original configuration blocks for Consul and Vault from
the server. When the server needs to refer to a Consul or Vault block it will
always be for a specific cluster for the task/service. Add a helper for
accessing the default clusters (for the servers own use).

This is one of three changesets for this work. The remainder will implement the
same changes in the `client` package and on the `command/agent` package.

As part of this work I discovered that the job submission hook for Vault only
checks the enabled flag on the default cluster, rather than the clusters that
are used by the job being submitted. This will return an error on job
registration saying that Vault is disabled. Fix that to check only the
cluster(s) used by the job.

Ref: #18947
Fixes: #18990
  • Loading branch information
tgross authored Nov 6, 2023
1 parent a13f0c6 commit 1ef99f0
Show file tree
Hide file tree
Showing 13 changed files with 121 additions and 108 deletions.
11 changes: 2 additions & 9 deletions command/agent/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -517,15 +517,8 @@ func convertServerConfig(agentConfig *Config) (*nomad.Config, error) {
}

// Add the Consul and Vault configs
conf.ConsulConfig = agentConfig.Consul
for _, consulConfig := range agentConfig.Consuls {
conf.ConsulConfigs[consulConfig.Name] = consulConfig
}

conf.VaultConfig = agentConfig.Vault
for _, vaultConfig := range agentConfig.Vaults {
conf.VaultConfigs[vaultConfig.Name] = vaultConfig
}
conf.ConsulConfigs = agentConfig.Consuls
conf.VaultConfigs = agentConfig.Vaults

// Set the TLS config
conf.TLSConfig = agentConfig.TLSConfig
Expand Down
59 changes: 30 additions & 29 deletions nomad/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -307,17 +307,11 @@ type Config struct {
// of all the heartbeats.
FailoverHeartbeatTTL time.Duration

// ConsulConfig is this Agent's Consul configuration
ConsulConfig *config.ConsulConfig

// ConsulConfigs is a map of Consul configurations, here to support features
// in Nomad Enterprise. The default Consul config pointer above will be
// found in this map under the name "default"
ConsulConfigs map[string]*config.ConsulConfig

// VaultConfig is this Agent's default Vault configuration
VaultConfig *config.VaultConfig

// VaultConfigs is a map of Vault configurations, here to support features
// in Nomad Enterprise. The default Vault config pointer above will be found
// in this map under the name "default"
Expand Down Expand Up @@ -457,9 +451,7 @@ func (c *Config) Copy() *Config {
nc.RaftConfig = pointer.Copy(c.RaftConfig)
nc.SerfConfig = pointer.Copy(c.SerfConfig)
nc.EnabledSchedulers = slices.Clone(c.EnabledSchedulers)
nc.ConsulConfig = c.ConsulConfig.Copy()
nc.ConsulConfigs = helper.DeepCopyMap(c.ConsulConfigs)
nc.VaultConfig = c.VaultConfig.Copy()
nc.VaultConfigs = helper.DeepCopyMap(c.VaultConfigs)
nc.TLSConfig = c.TLSConfig.Copy()
nc.SentinelConfig = c.SentinelConfig.Copy()
Expand All @@ -472,22 +464,24 @@ func (c *Config) Copy() *Config {

// ConsulServiceIdentity returns the workload identity to be used for accessing
// the Consul API to register and manage Consul services.
func (c *Config) ConsulServiceIdentity() *structs.WorkloadIdentity {
if c.ConsulConfig == nil {
func (c *Config) ConsulServiceIdentity(cluster string) *structs.WorkloadIdentity {
conf := c.ConsulConfigs[cluster]
if conf == nil {
return nil
}

return workloadIdentityFromConfig(c.ConsulConfig.ServiceIdentity)
return workloadIdentityFromConfig(conf.ServiceIdentity)
}

// ConsulTaskIdentity returns the workload identity to be used for accessing the
// Consul API from task hooks not supporting services (ex templates).
func (c *Config) ConsulTaskIdentity() *structs.WorkloadIdentity {
if c.ConsulConfig == nil {
func (c *Config) ConsulTaskIdentity(cluster string) *structs.WorkloadIdentity {
conf := c.ConsulConfigs[cluster]
if conf == nil {
return nil
}

return workloadIdentityFromConfig(c.ConsulConfig.TaskIdentity)
return workloadIdentityFromConfig(conf.TaskIdentity)
}

// VaultIdentityConfig returns the workload identity to be used for accessing
Expand All @@ -501,6 +495,14 @@ func (c *Config) VaultIdentityConfig(cluster string) *structs.WorkloadIdentity {
return workloadIdentityFromConfig(conf.DefaultIdentity)
}

func (c *Config) GetDefaultConsul() *config.ConsulConfig {
return c.ConsulConfigs[structs.ConsulDefaultCluster]
}

func (c *Config) GetDefaultVault() *config.VaultConfig {
return c.VaultConfigs[structs.VaultDefaultCluster]
}

// workloadIdentityFromConfig returns a structs.WorkloadIdentity to be used in
// a job from a config.WorkloadIdentityConfig parsed from an agent config file.
func workloadIdentityFromConfig(widConfig *config.WorkloadIdentityConfig) *structs.WorkloadIdentity {
Expand Down Expand Up @@ -581,18 +583,20 @@ func DefaultConfig() *Config {
NodePlanRejectionEnabled: false,
NodePlanRejectionThreshold: 15,
NodePlanRejectionWindow: 10 * time.Minute,
ConsulConfig: config.DefaultConsulConfig(),
VaultConfig: config.DefaultVaultConfig(),
RPCHoldTimeout: 5 * time.Second,
StatsCollectionInterval: 1 * time.Minute,
TLSConfig: &config.TLSConfig{},
ReplicationBackoff: 30 * time.Second,
SentinelGCInterval: 30 * time.Second,
LicenseConfig: &LicenseConfig{},
EnableEventBroker: true,
EventBufferSize: 100,
ACLTokenMinExpirationTTL: 1 * time.Minute,
ACLTokenMaxExpirationTTL: 24 * time.Hour,
ConsulConfigs: map[string]*config.ConsulConfig{
structs.ConsulDefaultCluster: config.DefaultConsulConfig()},
VaultConfigs: map[string]*config.VaultConfig{
structs.VaultDefaultCluster: config.DefaultVaultConfig()},
RPCHoldTimeout: 5 * time.Second,
StatsCollectionInterval: 1 * time.Minute,
TLSConfig: &config.TLSConfig{},
ReplicationBackoff: 30 * time.Second,
SentinelGCInterval: 30 * time.Second,
LicenseConfig: &LicenseConfig{},
EnableEventBroker: true,
EventBufferSize: 100,
ACLTokenMinExpirationTTL: 1 * time.Minute,
ACLTokenMaxExpirationTTL: 24 * time.Hour,
AutopilotConfig: &structs.AutopilotConfig{
CleanupDeadServers: true,
LastContactThreshold: 200 * time.Millisecond,
Expand All @@ -616,9 +620,6 @@ func DefaultConfig() *Config {
JobTrackedVersions: structs.JobDefaultTrackedVersions,
}

c.ConsulConfigs = map[string]*config.ConsulConfig{structs.ConsulDefaultCluster: c.ConsulConfig}
c.VaultConfigs = map[string]*config.VaultConfig{structs.VaultDefaultCluster: c.VaultConfig}

// Enable all known schedulers by default
c.EnabledSchedulers = make([]string, 0, len(scheduler.BuiltinSchedulers))
for name := range scheduler.BuiltinSchedulers {
Expand Down
2 changes: 1 addition & 1 deletion nomad/job_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ func (j *Job) Register(args *structs.JobRegisterRequest, reply *structs.JobRegis
// - reading kv store of each group
// - establishing consul connect services
checkConsulToken := func(usages map[string]*structs.ConsulUsage) error {
if j.srv.config.ConsulConfig.AllowsUnauthenticated() {
if j.srv.config.GetDefaultConsul().AllowsUnauthenticated() {
// if consul.allow_unauthenticated is enabled (which is the default)
// just let the job through without checking anything
return nil
Expand Down
2 changes: 1 addition & 1 deletion nomad/job_endpoint_ce_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func TestJobEndpoint_Register_Connect_AllowUnauthenticatedFalse_oss(t *testing.T

s1, cleanupS1 := TestServer(t, func(c *Config) {
c.NumSchedulers = 0 // Prevent automatic dequeue
c.ConsulConfig.AllowUnauthenticated = pointer.Of(false)
c.GetDefaultConsul().AllowUnauthenticated = pointer.Of(false)
})
defer cleanupS1()
codec := rpcClient(t, s1)
Expand Down
16 changes: 8 additions & 8 deletions nomad/job_endpoint_hook_implicit_identities.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,15 @@ func (jobImplicitIdentitiesHook) Name() string {
func (h jobImplicitIdentitiesHook) Mutate(job *structs.Job) (*structs.Job, []error, error) {
for _, tg := range job.TaskGroups {
for _, s := range tg.Services {
h.handleConsulService(s)
h.handleConsulService(s, tg)
}

for _, t := range tg.Tasks {
for _, s := range t.Services {
h.handleConsulService(s)
h.handleConsulService(s, tg)
}
if len(t.Templates) > 0 {
h.handleConsulTasks(t)
h.handleConsulTasks(t, tg)
}
h.handleVault(t)
}
Expand All @@ -43,7 +43,7 @@ func (h jobImplicitIdentitiesHook) Mutate(job *structs.Job) (*structs.Job, []err
//
// If the service already has an identity the server sets the identity name and
// service name values.
func (h jobImplicitIdentitiesHook) handleConsulService(s *structs.Service) {
func (h jobImplicitIdentitiesHook) handleConsulService(s *structs.Service, tg *structs.TaskGroup) {
if s.Provider != "" && s.Provider != "consul" {
return
}
Expand All @@ -53,7 +53,7 @@ func (h jobImplicitIdentitiesHook) handleConsulService(s *structs.Service) {
if serviceWID == nil {
// If the service doesn't specify an identity, fallback to the service
// identity defined in the server configuration.
serviceWID = h.srv.config.ConsulServiceIdentity()
serviceWID = h.srv.config.ConsulServiceIdentity(s.GetConsulClusterName(tg))
if serviceWID == nil {
// If no identity is found, skip injecting the implicit identity
// and fallback to the legacy flow.
Expand All @@ -68,7 +68,7 @@ func (h jobImplicitIdentitiesHook) handleConsulService(s *structs.Service) {
s.Identity = serviceWID
}

func (h jobImplicitIdentitiesHook) handleConsulTasks(t *structs.Task) {
func (h jobImplicitIdentitiesHook) handleConsulTasks(t *structs.Task, tg *structs.TaskGroup) {
widName := t.Consul.IdentityName()

// Use the Consul identity specified in the task if present
Expand All @@ -80,7 +80,7 @@ func (h jobImplicitIdentitiesHook) handleConsulTasks(t *structs.Task) {

// If task doesn't specify an identity for Consul, fallback to the
// default identity defined in the server configuration.
taskWID := h.srv.config.ConsulTaskIdentity()
taskWID := h.srv.config.ConsulTaskIdentity(t.GetConsulClusterName(tg))
if taskWID == nil {
// If no identity is found skip inject the implicit identity and
// fallback to the legacy flow.
Expand Down Expand Up @@ -108,7 +108,7 @@ func (h jobImplicitIdentitiesHook) handleVault(t *structs.Task) {

// If the task doesn't specify an identity for Vault, fallback to the
// default identity defined in the server configuration.
vaultWID = h.srv.config.VaultIdentityConfig(t.Vault.Cluster)
vaultWID = h.srv.config.VaultIdentityConfig(t.GetVaultClusterName())
if vaultWID == nil {
// If no identity is found skip inject the implicit identity and
// fallback to the legacy flow.
Expand Down
39 changes: 23 additions & 16 deletions nomad/job_endpoint_hook_implicit_identities_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ func Test_jobImplicitIdentitiesHook_Mutate_consul_service(t *testing.T) {
}},
},
inputConfig: &Config{
ConsulConfig: &config.ConsulConfig{},
ConsulConfigs: map[string]*config.ConsulConfig{},
},
expectedOutputJob: &structs.Job{
TaskGroups: []*structs.TaskGroup{{
Expand All @@ -51,11 +51,12 @@ func Test_jobImplicitIdentitiesHook_Mutate_consul_service(t *testing.T) {
}},
},
inputConfig: &Config{
ConsulConfig: &config.ConsulConfig{
ServiceIdentity: &config.WorkloadIdentityConfig{
Audience: []string{"consul.io"},
},
},
ConsulConfigs: map[string]*config.ConsulConfig{
structs.ConsulDefaultCluster: {
ServiceIdentity: &config.WorkloadIdentityConfig{
Audience: []string{"consul.io"},
},
}},
},
expectedOutputJob: &structs.Job{
TaskGroups: []*structs.TaskGroup{{
Expand Down Expand Up @@ -108,9 +109,11 @@ func Test_jobImplicitIdentitiesHook_Mutate_consul_service(t *testing.T) {
}},
},
inputConfig: &Config{
ConsulConfig: &config.ConsulConfig{
ServiceIdentity: &config.WorkloadIdentityConfig{
Audience: []string{"consul.io"},
ConsulConfigs: map[string]*config.ConsulConfig{
structs.ConsulDefaultCluster: {
ServiceIdentity: &config.WorkloadIdentityConfig{
Audience: []string{"consul.io"},
},
},
},
},
Expand Down Expand Up @@ -182,9 +185,11 @@ func Test_jobImplicitIdentitiesHook_Mutate_consul_service(t *testing.T) {
}},
},
inputConfig: &Config{
ConsulConfig: &config.ConsulConfig{
ServiceIdentity: &config.WorkloadIdentityConfig{
Audience: []string{"consul.io"},
ConsulConfigs: map[string]*config.ConsulConfig{
structs.ConsulDefaultCluster: {
ServiceIdentity: &config.WorkloadIdentityConfig{
Audience: []string{"consul.io"},
},
},
},
},
Expand Down Expand Up @@ -229,9 +234,11 @@ func Test_jobImplicitIdentitiesHook_Mutate_consul_service(t *testing.T) {
}},
},
inputConfig: &Config{
ConsulConfig: &config.ConsulConfig{
TaskIdentity: &config.WorkloadIdentityConfig{
Audience: []string{"consul.io"},
ConsulConfigs: map[string]*config.ConsulConfig{
structs.ConsulDefaultCluster: {
TaskIdentity: &config.WorkloadIdentityConfig{
Audience: []string{"consul.io"},
},
},
},
},
Expand Down Expand Up @@ -260,7 +267,7 @@ func Test_jobImplicitIdentitiesHook_Mutate_consul_service(t *testing.T) {
}},
},
inputConfig: &Config{
ConsulConfig: &config.ConsulConfig{},
ConsulConfigs: map[string]*config.ConsulConfig{},
},
expectedOutputJob: &structs.Job{
TaskGroups: []*structs.TaskGroup{{
Expand Down
17 changes: 13 additions & 4 deletions nomad/job_endpoint_hook_vault.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,18 @@ func (h jobVaultHook) Validate(job *structs.Job) ([]error, error) {
return nil, nil
}

vconf := h.srv.config.VaultConfig
if !vconf.IsEnabled() {
return nil, fmt.Errorf("Vault not enabled but used in the job")
requiresToken := false
for _, tg := range vaultBlocks {
for _, vaultBlock := range tg {
vconf := h.srv.config.VaultConfigs[vaultBlock.Cluster]
if !vconf.IsEnabled() {
return nil, fmt.Errorf("Vault %q not enabled but used in the job",
vaultBlock.Cluster)
}
if !vconf.AllowsUnauthenticated() {
requiresToken = true
}
}
}

err := h.validateClustersForNamespace(job, vaultBlocks)
Expand All @@ -40,7 +49,7 @@ func (h jobVaultHook) Validate(job *structs.Job) ([]error, error) {
}

// Return early if Vault configuration doesn't require authentication.
if vconf.AllowsUnauthenticated() {
if !requiresToken {
return nil, nil
}

Expand Down
16 changes: 9 additions & 7 deletions nomad/job_endpoint_hooks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func Test_jobValidate_Validate_consul_service(t *testing.T) {
Name: "web",
},
inputConfig: &Config{
ConsulConfig: &config.ConsulConfig{},
ConsulConfigs: map[string]*config.ConsulConfig{},
},
},
{
Expand All @@ -43,10 +43,12 @@ func Test_jobValidate_Validate_consul_service(t *testing.T) {
Name: "web",
},
inputConfig: &Config{
ConsulConfig: &config.ConsulConfig{
ServiceIdentity: &config.WorkloadIdentityConfig{
Audience: []string{"consul.io"},
TTL: pointer.Of(time.Hour),
ConsulConfigs: map[string]*config.ConsulConfig{
structs.ConsulDefaultCluster: {
ServiceIdentity: &config.WorkloadIdentityConfig{
Audience: []string{"consul.io"},
TTL: pointer.Of(time.Hour),
},
},
},
},
Expand All @@ -66,7 +68,7 @@ func Test_jobValidate_Validate_consul_service(t *testing.T) {
},
},
inputConfig: &Config{
ConsulConfig: &config.ConsulConfig{},
ConsulConfigs: map[string]*config.ConsulConfig{},
},
},
{
Expand All @@ -83,7 +85,7 @@ func Test_jobValidate_Validate_consul_service(t *testing.T) {
},
},
inputConfig: &Config{
ConsulConfig: &config.ConsulConfig{},
ConsulConfigs: map[string]*config.ConsulConfig{},
},
expectedWarns: []string{
"identities without an expiration are insecure",
Expand Down
Loading

0 comments on commit 1ef99f0

Please sign in to comment.