From cc07256bb5e2de9a15ff21c32227a0877feadd58 Mon Sep 17 00:00:00 2001 From: Preetha Appan Date: Thu, 28 Mar 2019 16:30:38 -0500 Subject: [PATCH] Fix json parsing bug with plugins that don't provide args This fixes a bug with JSON agent configuration parsing where the AST for the plugin stanza had unnecessary flattening originating from hcl parsing library. The workaround fixes the AST by popping off the flattened element and wrapping it in a list. The workaround comes from similar code in terraform. There were no existing test cases for json parsing so I added a few. --- command/agent/config-test-fixtures/basic.json | 288 ++++++++++++++ command/agent/config-test-fixtures/plugin.hcl | 13 + .../agent/config-test-fixtures/plugin.json | 17 + command/agent/config_parse.go | 44 +++ command/agent/config_parse_test.go | 359 ++++++++++++++++++ 5 files changed, 721 insertions(+) create mode 100644 command/agent/config-test-fixtures/basic.json create mode 100644 command/agent/config-test-fixtures/plugin.hcl create mode 100644 command/agent/config-test-fixtures/plugin.json diff --git a/command/agent/config-test-fixtures/basic.json b/command/agent/config-test-fixtures/basic.json new file mode 100644 index 000000000..ef1ec6dba --- /dev/null +++ b/command/agent/config-test-fixtures/basic.json @@ -0,0 +1,288 @@ +{ + "acl": [ + { + "enabled": true, + "policy_ttl": "60s", + "replication_token": "foobar", + "token_ttl": "60s" + } + ], + "addresses": [ + { + "http": "127.0.0.1", + "rpc": "127.0.0.2", + "serf": "127.0.0.3" + } + ], + "advertise": [ + { + "rpc": "127.0.0.3", + "serf": "127.0.0.4" + } + ], + "autopilot": [ + { + "cleanup_dead_servers": true, + "disable_upgrade_migration": true, + "enable_custom_upgrades": true, + "enable_redundancy_zones": true, + "last_contact_threshold": "12705s", + "max_trailing_logs": 17849, + "server_stabilization_time": "23057s" + } + ], + "bind_addr": "192.168.0.1", + "client": [ + { + "alloc_dir": "/tmp/alloc", + "chroot_env": [ + { + "/opt/myapp/bin": "/bin", + "/opt/myapp/etc": "/etc" + } + ], + "client_max_port": 2000, + "client_min_port": 1000, + "cpu_total_compute": 4444, + "enabled": true, + "gc_disk_usage_threshold": 82, + "gc_inode_usage_threshold": 91, + "gc_interval": "6s", + "gc_max_allocs": 50, + "gc_parallel_destroys": 6, + "max_kill_timeout": "10s", + "meta": [ + { + "baz": "zip", + "foo": "bar" + } + ], + "network_interface": "eth0", + "network_speed": 100, + "no_host_uuid": false, + "node_class": "linux-medium-64bit", + "options": [ + { + "baz": "zip", + "foo": "bar" + } + ], + "reserved": [ + { + "cpu": 10, + "disk": 10, + "memory": 10, + "reserved_ports": "1,100,10-12" + } + ], + "server_join": [ + { + "retry_interval": "15s", + "retry_join": [ + "1.1.1.1", + "2.2.2.2" + ], + "retry_max": 3 + } + ], + "servers": [ + "a.b.c:80", + "127.0.0.1:1234" + ], + "state_dir": "/tmp/client-state", + "stats": [ + { + "collection_interval": "5s", + "data_points": 35 + } + ] + } + ], + "consul": [ + { + "address": "127.0.0.1:9500", + "auth": "username:pass", + "auto_advertise": true, + "ca_file": "/path/to/ca/file", + "cert_file": "/path/to/cert/file", + "checks_use_advertise": true, + "client_auto_join": true, + "client_http_check_name": "nomad-client-http-health-check", + "client_service_name": "nomad-client", + "key_file": "/path/to/key/file", + "server_auto_join": true, + "server_http_check_name": "nomad-server-http-health-check", + "server_rpc_check_name": "nomad-server-rpc-health-check", + "server_serf_check_name": "nomad-server-serf-health-check", + "server_service_name": "nomad", + "ssl": true, + "token": "token1", + "verify_ssl": true + } + ], + "data_dir": "/tmp/nomad", + "datacenter": "dc2", + "disable_anonymous_signature": true, + "disable_update_check": true, + "enable_debug": true, + "enable_syslog": true, + "http_api_response_headers": [ + { + "Access-Control-Allow-Origin": "*" + } + ], + "leave_on_interrupt": true, + "leave_on_terminate": true, + "log_json": true, + "log_level": "ERR", + "name": "my-web", + "plugin": { + "docker": { + "args": [ + "foo", + "bar" + ], + "config": { + "foo": "bar", + "nested": { + "bam": 2 + } + } + }, + "exec": { + "config": { + "foo": true + } + } + }, + "plugin_dir": "/tmp/nomad-plugins", + "ports": [ + { + "http": 1234, + "rpc": 2345, + "serf": 3456 + } + ], + "region": "foobar", + "sentinel": [ + { + "import": [ + { + "foo": [ + { + "args": [ + "a", + "b", + "c" + ], + "path": "foo" + } + ] + }, + { + "bar": [ + { + "args": [ + "x", + "y", + "z" + ], + "path": "bar" + } + ] + } + ] + } + ], + "server": [ + { + "authoritative_region": "foobar", + "bootstrap_expect": 5, + "data_dir": "/tmp/data", + "deployment_gc_threshold": "12h", + "enabled": true, + "enabled_schedulers": [ + "test" + ], + "encrypt": "abc", + "eval_gc_threshold": "12h", + "heartbeat_grace": "30s", + "job_gc_threshold": "12h", + "max_heartbeats_per_second": 11, + "min_heartbeat_ttl": "33s", + "node_gc_threshold": "12h", + "non_voting_server": true, + "num_schedulers": 2, + "protocol_version": 3, + "raft_protocol": 3, + "redundancy_zone": "foo", + "rejoin_after_leave": true, + "retry_interval": "15s", + "retry_join": [ + "1.1.1.1", + "2.2.2.2" + ], + "retry_max": 3, + "server_join": [ + { + "retry_interval": "15s", + "retry_join": [ + "1.1.1.1", + "2.2.2.2" + ], + "retry_max": 3 + } + ], + "start_join": [ + "1.1.1.1", + "2.2.2.2" + ], + "upgrade_version": "0.8.0" + } + ], + "syslog_facility": "LOCAL1", + "telemetry": [ + { + "backwards_compatible_metrics": true, + "collection_interval": "3s", + "disable_hostname": true, + "disable_tagged_metrics": true, + "prometheus_metrics": true, + "publish_allocation_metrics": true, + "publish_node_metrics": true, + "statsd_address": "127.0.0.1:2345", + "statsite_address": "127.0.0.1:1234" + } + ], + "tls": [ + { + "ca_file": "foo", + "cert_file": "bar", + "http": true, + "key_file": "pipe", + "rpc": true, + "rpc_upgrade_mode": true, + "tls_cipher_suites": "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256", + "tls_min_version": "tls12", + "tls_prefer_server_cipher_suites": true, + "verify_https_client": true, + "verify_server_hostname": true + } + ], + "vault": [ + { + "address": "127.0.0.1:9500", + "allow_unauthenticated": true, + "ca_file": "/path/to/ca/file", + "ca_path": "/path/to/ca", + "cert_file": "/path/to/cert/file", + "create_from_role": "test_role", + "enabled": false, + "key_file": "/path/to/key/file", + "task_token_ttl": "1s", + "tls_server_name": "foobar", + "tls_skip_verify": true, + "token": "12345" + } + ] +} \ No newline at end of file diff --git a/command/agent/config-test-fixtures/plugin.hcl b/command/agent/config-test-fixtures/plugin.hcl new file mode 100644 index 000000000..f6ccc59e2 --- /dev/null +++ b/command/agent/config-test-fixtures/plugin.hcl @@ -0,0 +1,13 @@ +client { + memory_total_mb = 5555 +} +plugin "docker" { + config { + allow_privileged = true + } +} +plugin "raw_exec" { + config { + enabled = true + } +} diff --git a/command/agent/config-test-fixtures/plugin.json b/command/agent/config-test-fixtures/plugin.json new file mode 100644 index 000000000..703daaf10 --- /dev/null +++ b/command/agent/config-test-fixtures/plugin.json @@ -0,0 +1,17 @@ +{ + "client" : { + "memory_total_mb": 5555 + }, + "plugin": { + "docker": { + "config": { + "allow_privileged": true + } + }, + "raw_exec": { + "config": { + "enabled": true + } + } + } +} \ No newline at end of file diff --git a/command/agent/config_parse.go b/command/agent/config_parse.go index 6df9e83d4..504dddbbc 100644 --- a/command/agent/config_parse.go +++ b/command/agent/config_parse.go @@ -1014,6 +1014,11 @@ func parsePlugins(result *[]*config.PluginConfig, list *ast.ObjectList) error { // Get the current plugin object listVal := list.Items[i] + // Deal with json->hcl AST parsing incorrectness when directly nested + // items show up as additional keys. This currently only affects plugin + // configuration because args is not necessary. All other fields in the config + // have multiple keys and parse from json into the AST correctly. + unwrapLegacyHCLObjectKeysFromJSON(listVal, 1) if err := helper.CheckHCLKeys(listVal.Val, valid); err != nil { return fmt.Errorf("invalid keys in plugin config %d: %v", i+1, err) } @@ -1034,3 +1039,42 @@ func parsePlugins(result *[]*config.PluginConfig, list *ast.ObjectList) error { *result = plugins return nil } + +// unwrapLegacyHCLObjectKeysFromJSON cleans up an edge case that can occur when +// parsing JSON as input: if we're parsing JSON then directly nested +// items will show up as additional "keys". +// +// For objects that expect a fixed number of keys, this breaks the +// decoding process. This function unwraps the object into what it would've +// looked like if it came directly from HCL by specifying the number of keys +// you expect. +// +// Example: +// +// { "foo": { "baz": {} } } +// +// Will show up with Keys being: []string{"foo", "baz"} +// when we really just want the first two. This function will fix this. +func unwrapLegacyHCLObjectKeysFromJSON(item *ast.ObjectItem, depth int) { + if len(item.Keys) > depth && item.Keys[0].Token.JSON { + for len(item.Keys) > depth { + // Pop off the last key + n := len(item.Keys) + key := item.Keys[n-1] + item.Keys[n-1] = nil + item.Keys = item.Keys[:n-1] + + // Wrap our value in a list + item.Val = &ast.ObjectType{ + List: &ast.ObjectList{ + Items: []*ast.ObjectItem{ + &ast.ObjectItem{ + Keys: []*ast.ObjectKey{key}, + Val: item.Val, + }, + }, + }, + } + } + } +} diff --git a/command/agent/config_parse_test.go b/command/agent/config_parse_test.go index 3ead69cf1..84b0de9e0 100644 --- a/command/agent/config_parse_test.go +++ b/command/agent/config_parse_test.go @@ -238,6 +238,365 @@ func TestConfig_Parse(t *testing.T) { }, false, }, + { + "basic.json", + &Config{ + Region: "foobar", + Datacenter: "dc2", + NodeName: "my-web", + DataDir: "/tmp/nomad", + PluginDir: "/tmp/nomad-plugins", + LogLevel: "ERR", + LogJson: true, + BindAddr: "192.168.0.1", + EnableDebug: true, + Ports: &Ports{ + HTTP: 1234, + RPC: 2345, + Serf: 3456, + }, + Addresses: &Addresses{ + HTTP: "127.0.0.1", + RPC: "127.0.0.2", + Serf: "127.0.0.3", + }, + AdvertiseAddrs: &AdvertiseAddrs{ + RPC: "127.0.0.3", + Serf: "127.0.0.4", + }, + Client: &ClientConfig{ + Enabled: true, + StateDir: "/tmp/client-state", + AllocDir: "/tmp/alloc", + Servers: []string{"a.b.c:80", "127.0.0.1:1234"}, + NodeClass: "linux-medium-64bit", + ServerJoin: &ServerJoin{ + RetryJoin: []string{"1.1.1.1", "2.2.2.2"}, + RetryInterval: time.Duration(15) * time.Second, + RetryMaxAttempts: 3, + }, + Meta: map[string]string{ + "foo": "bar", + "baz": "zip", + }, + Options: map[string]string{ + "foo": "bar", + "baz": "zip", + }, + ChrootEnv: map[string]string{ + "/opt/myapp/etc": "/etc", + "/opt/myapp/bin": "/bin", + }, + NetworkInterface: "eth0", + NetworkSpeed: 100, + CpuCompute: 4444, + MemoryMB: 0, + MaxKillTimeout: "10s", + ClientMinPort: 1000, + ClientMaxPort: 2000, + Reserved: &Resources{ + CPU: 10, + MemoryMB: 10, + DiskMB: 10, + ReservedPorts: "1,100,10-12", + }, + GCInterval: 6 * time.Second, + GCParallelDestroys: 6, + GCDiskUsageThreshold: 82, + GCInodeUsageThreshold: 91, + GCMaxAllocs: 50, + NoHostUUID: helper.BoolToPtr(false), + }, + Server: &ServerConfig{ + Enabled: true, + AuthoritativeRegion: "foobar", + BootstrapExpect: 5, + DataDir: "/tmp/data", + ProtocolVersion: 3, + RaftProtocol: 3, + NumSchedulers: helper.IntToPtr(2), + EnabledSchedulers: []string{"test"}, + NodeGCThreshold: "12h", + EvalGCThreshold: "12h", + JobGCThreshold: "12h", + DeploymentGCThreshold: "12h", + HeartbeatGrace: 30 * time.Second, + MinHeartbeatTTL: 33 * time.Second, + MaxHeartbeatsPerSecond: 11.0, + RetryJoin: []string{"1.1.1.1", "2.2.2.2"}, + StartJoin: []string{"1.1.1.1", "2.2.2.2"}, + RetryInterval: 15 * time.Second, + RejoinAfterLeave: true, + RetryMaxAttempts: 3, + NonVotingServer: true, + RedundancyZone: "foo", + UpgradeVersion: "0.8.0", + EncryptKey: "abc", + ServerJoin: &ServerJoin{ + RetryJoin: []string{"1.1.1.1", "2.2.2.2"}, + RetryInterval: time.Duration(15) * time.Second, + RetryMaxAttempts: 3, + }, + }, + ACL: &ACLConfig{ + Enabled: true, + TokenTTL: 60 * time.Second, + PolicyTTL: 60 * time.Second, + ReplicationToken: "foobar", + }, + Telemetry: &Telemetry{ + StatsiteAddr: "127.0.0.1:1234", + StatsdAddr: "127.0.0.1:2345", + PrometheusMetrics: true, + DisableHostname: true, + UseNodeName: false, + CollectionInterval: "3s", + collectionInterval: 3 * time.Second, + PublishAllocationMetrics: true, + PublishNodeMetrics: true, + DisableTaggedMetrics: true, + BackwardsCompatibleMetrics: true, + }, + LeaveOnInt: true, + LeaveOnTerm: true, + EnableSyslog: true, + SyslogFacility: "LOCAL1", + DisableUpdateCheck: helper.BoolToPtr(true), + DisableAnonymousSignature: true, + Consul: &config.ConsulConfig{ + ServerServiceName: "nomad", + ServerHTTPCheckName: "nomad-server-http-health-check", + ServerSerfCheckName: "nomad-server-serf-health-check", + ServerRPCCheckName: "nomad-server-rpc-health-check", + ClientServiceName: "nomad-client", + ClientHTTPCheckName: "nomad-client-http-health-check", + Addr: "127.0.0.1:9500", + Token: "token1", + Auth: "username:pass", + EnableSSL: &trueValue, + VerifySSL: &trueValue, + CAFile: "/path/to/ca/file", + CertFile: "/path/to/cert/file", + KeyFile: "/path/to/key/file", + ServerAutoJoin: &trueValue, + ClientAutoJoin: &trueValue, + AutoAdvertise: &trueValue, + ChecksUseAdvertise: &trueValue, + }, + Vault: &config.VaultConfig{ + Addr: "127.0.0.1:9500", + AllowUnauthenticated: &trueValue, + Enabled: &falseValue, + Role: "test_role", + TLSCaFile: "/path/to/ca/file", + TLSCaPath: "/path/to/ca", + TLSCertFile: "/path/to/cert/file", + TLSKeyFile: "/path/to/key/file", + TLSServerName: "foobar", + TLSSkipVerify: &trueValue, + TaskTokenTTL: "1s", + Token: "12345", + }, + TLSConfig: &config.TLSConfig{ + EnableHTTP: true, + EnableRPC: true, + VerifyServerHostname: true, + CAFile: "foo", + CertFile: "bar", + KeyFile: "pipe", + RPCUpgradeMode: true, + VerifyHTTPSClient: true, + TLSPreferServerCipherSuites: true, + TLSCipherSuites: "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256", + TLSMinVersion: "tls12", + }, + HTTPAPIResponseHeaders: map[string]string{ + "Access-Control-Allow-Origin": "*", + }, + Sentinel: &config.SentinelConfig{ + Imports: []*config.SentinelImport{ + { + Name: "foo", + Path: "foo", + Args: []string{"a", "b", "c"}, + }, + { + Name: "bar", + Path: "bar", + Args: []string{"x", "y", "z"}, + }, + }, + }, + Autopilot: &config.AutopilotConfig{ + CleanupDeadServers: &trueValue, + ServerStabilizationTime: 23057 * time.Second, + LastContactThreshold: 12705 * time.Second, + MaxTrailingLogs: 17849, + EnableRedundancyZones: &trueValue, + DisableUpgradeMigration: &trueValue, + EnableCustomUpgrades: &trueValue, + }, + Plugins: []*config.PluginConfig{ + { + Name: "docker", + Args: []string{"foo", "bar"}, + Config: map[string]interface{}{ + "foo": "bar", + "nested": []map[string]interface{}{ + { + "bam": 2, + }, + }, + }, + }, + { + Name: "exec", + Config: map[string]interface{}{ + "foo": true, + }, + }, + }, + }, + false, + }, + { + "plugin.hcl", + &Config{ + Region: "", + Datacenter: "", + NodeName: "", + DataDir: "", + PluginDir: "", + LogLevel: "", + BindAddr: "", + EnableDebug: false, + Ports: nil, + Addresses: nil, + AdvertiseAddrs: nil, + Client: &ClientConfig{ + Enabled: false, + StateDir: "", + AllocDir: "", + Servers: nil, + NodeClass: "", + Meta: nil, + Options: nil, + ChrootEnv: nil, + NetworkInterface: "", + NetworkSpeed: 0, + CpuCompute: 0, + MemoryMB: 5555, + MaxKillTimeout: "", + ClientMinPort: 0, + ClientMaxPort: 0, + Reserved: nil, + GCInterval: 0, + GCParallelDestroys: 0, + GCDiskUsageThreshold: 0, + GCInodeUsageThreshold: 0, + GCMaxAllocs: 0, + NoHostUUID: nil, + }, + Server: nil, + ACL: nil, + Telemetry: nil, + LeaveOnInt: false, + LeaveOnTerm: false, + EnableSyslog: false, + SyslogFacility: "", + DisableUpdateCheck: nil, + DisableAnonymousSignature: false, + Consul: nil, + Vault: nil, + TLSConfig: nil, + HTTPAPIResponseHeaders: nil, + Sentinel: nil, + Plugins: []*config.PluginConfig{ + { + Name: "docker", + Config: map[string]interface{}{ + "allow_privileged": true, + }, + }, + { + Name: "raw_exec", + Config: map[string]interface{}{ + "enabled": true, + }, + }, + }, + }, + false, + }, + { + "plugin.json", + &Config{ + Region: "", + Datacenter: "", + NodeName: "", + DataDir: "", + PluginDir: "", + LogLevel: "", + BindAddr: "", + EnableDebug: false, + Ports: nil, + Addresses: nil, + AdvertiseAddrs: nil, + Client: &ClientConfig{ + Enabled: false, + StateDir: "", + AllocDir: "", + Servers: nil, + NodeClass: "", + Meta: nil, + Options: nil, + ChrootEnv: nil, + NetworkInterface: "", + NetworkSpeed: 0, + CpuCompute: 0, + MemoryMB: 5555, + MaxKillTimeout: "", + ClientMinPort: 0, + ClientMaxPort: 0, + Reserved: nil, + GCInterval: 0, + GCParallelDestroys: 0, + GCDiskUsageThreshold: 0, + GCInodeUsageThreshold: 0, + GCMaxAllocs: 0, + NoHostUUID: nil, + }, + Server: nil, + ACL: nil, + Telemetry: nil, + LeaveOnInt: false, + LeaveOnTerm: false, + EnableSyslog: false, + SyslogFacility: "", + DisableUpdateCheck: nil, + DisableAnonymousSignature: false, + Consul: nil, + Vault: nil, + TLSConfig: nil, + HTTPAPIResponseHeaders: nil, + Sentinel: nil, + Plugins: []*config.PluginConfig{ + { + Name: "docker", + Config: map[string]interface{}{ + "allow_privileged": true, + }, + }, + { + Name: "raw_exec", + Config: map[string]interface{}{ + "enabled": true, + }, + }, + }, + }, + false, + }, { "non-optional.hcl", &Config{