hashicorp / consul

Consul is a distributed, highly available, and data center aware solution to connect and configure applications across dynamic, distributed infrastructure.
https://www.consul.io
Other
28.37k stars 4.42k forks source link

Consul validate fails to catch syntax errors JSON configs #13793

Open ramramhariram opened 2 years ago

ramramhariram commented 2 years ago

Consul validate did not catch a missing "comma" that caused Consul validation to pass but agent parsing failed and caused federation issues.

Examples below but please note - In this specific scenario, the configs were json but user named the file as .hcl (Shouldn't consul validate catch this regardless?)

#########Full Configuration sample (sensitive data redacted/replaced)#########

{ "log_level": "DEBUG", "log_file": "/opt/consul/", "server": true, "node_name": "xxxx-cluster-VM-2", "ui": true, "leave_on_terminate": true, "data_dir": "/opt/consul/data", "datacenter": "BBBBB", "alt_domain": "xxxx.com", "client_addr": "0.0.0.0", "bind_addr": "10.x.x.x", "bootstrap_expect": 5, "advertise_addr" : "10.x.x.x", "retry_join" : ["10.x.x.x", "10.x.x.x", "10.x.x.x", "10.x.x.x"], "enable_syslog": true, "verify_incoming": true, "verify_outgoing": true, "verify_server_hostname": true, "ca_file": "/etc/consul.d/consul-agent-ca.pem", "cert_file": "/etc/consul.d/bbbb-server-consul-1.pem", "key_file": "/etc/consul.d/bbbb-server-consul-1-key.pem", "encrypt": "xxxxxx", "auto_encrypt": { "allow_tls": true }, "connect": { "enabled": true }, "performance": { "raft_multiplier": 1 } "primary_datacenter" : "AAAAA", "retry_join_wan" : ["110.x.x.x", "110.x.x.x", "10.x.x.x", "10.x.x.x", "10.x.x.x"] } #########Syntax issue section only#########

"performance": { "raft_multiplier": 1 } "primary_datacenter" : "AAAAA",

#########Consul validate output######## [xxx-server-vm-1 ~]$ consul validate /etc/consul.d/consul.hcl The 'ui' field is deprecated. Use the 'ui_config.enabled' field instead. bootstrap_expect > 0: expecting 5 servers Configuration is valid!

[xxx-server-vm-1 ~]$

#########consul version#########

[xxx-server-vm-1 ~]$ consul version Consul v1.10.3 Revision c976ffd2d Protocol 2 spoken by default, understands 2 to 3 (agent will automatically use protocol >2 when speaking to compatible agents)

[xxx-server-vm-1 ~]$

ramramhariram commented 2 years ago

Updated with some more information

huikang commented 2 years ago

I can reproduce the same behavior with the main branch. I doubt if this is caused by the very old hcl package (v1.0.0) used in consul.

elee commented 1 year ago

👋 @ramramhariram Attempting to reproduce this in a test case in validate_test.go I added the following test based on your hcl:

func TestValidateOnInvalidJSONHCL(t *testing.T) {
    t.Parallel()
    td := testutil.TempDir(t, "jsonhclerror")

    fp := filepath.Join(td, "foo.json")
    err := os.WriteFile(fp, []byte(`{
"log_level": "DEBUG",
"log_file": "/opt/consul/",
"server": true,
"node_name": "xxxx-cluster-VM-2",
"ui": true,
"leave_on_terminate": true,
"data_dir": "/opt/consul/data",
"datacenter": "BBBBB",
"alt_domain": "xxxx.com",
"client_addr": "0.0.0.0",
"bind_addr": "10.10.10.10",
"bootstrap_expect": 5,
"advertise_addr" : "10.10.10.10",
"retry_join" : ["10.10.10.10", "10.10.10.10", "10.10.10.10", "10.10.10.10"],
"enable_syslog": true,
"verify_incoming": true,
"verify_outgoing": true,
"verify_server_hostname": true,
"auto_encrypt": {
"allow_tls": true
},
"connect": {
"enabled": true
},
"performance": {
"raft_multiplier": 1
}
"primary_datacenter" : "AAAAA",
"retry_join_wan" : ["110.10.10.10", "110.10.10.10", "10.10.10.10", "10.10.10.10", "10.10.10.10"]
}
`), 0644)
    require.NoError(t, err, "error creating file")
    ui := cli.NewMockUi()

    cmd := New(ui)
    args := []string{"-quiet", td}

    code := cmd.Run(args)
    require.Equal(t, 1, code)
    require.Equal(t, "fooby wooby", ui.ErrorWriter.String())
}

This produces different errors based on whether there is an hcl extension or json extension -- in the hcl case it produces an error about provided values being invalid and in the json case it produces a parse error. Based on this it seems like the behavior of consul validate is valid -- there's nothing but the extension of the file for the validate command to interpret the input. I think this Issue should be closed as a non-bug