Skip to content

Commit

Permalink
Merge pull request #10931 from hashicorp/b-hcl1-backport-fields
Browse files Browse the repository at this point in the history
Support the new post-1.0.0 job spec fields in the HCLv1 parser.

The HCLv1 parser is still the default (or only!) parser in many downstream tools, e.g. [Levant](https://github.com/hashicorp/levant/blob/e48c439f14def83b0cbe24c2553e90e959474f16/template/render.go#L13-L30), and [terraform-provider-nomad](https://github.com/hashicorp/terraform-provider-nomad/blob/bce32a783176c7943ac662a3eee48b76d92c4d6a/nomad/resource_job.go#L735-L743) .

While we initially intended to deprecate HCLv1 parser in 1.0.0, we never communicated that publicly. We did not fully anticipate the public usage of `jobspec` package (we assumed it's a private package), or the friction that HCLv2 introduced in some cases (e.g. #10777, #9838).

So moving forward we intend to ensure that new job spec fields are honored in both the HCLv1 and HCLv2 parser until we solidify the migration path and communicate it properly.
  • Loading branch information
Mahmood Ali committed Jul 26, 2021
2 parents 44ea61e + a4e9c11 commit e5a4dde
Show file tree
Hide file tree
Showing 9 changed files with 192 additions and 22 deletions.
81 changes: 72 additions & 9 deletions jobspec/parse_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ func parseService(o *ast.ObjectItem) (*api.Service, error) {
"task",
"meta",
"canary_meta",
"on_update",
}
if err := checkHCLKeys(o.Val, valid); err != nil {
return nil, err
Expand Down Expand Up @@ -226,6 +227,7 @@ func parseGateway(o *ast.ObjectItem) (*api.ConsulGateway, error) {
"proxy",
"ingress",
"terminating",
"mesh",
}

if err := checkHCLKeys(o.Val, valid); err != nil {
Expand All @@ -241,6 +243,7 @@ func parseGateway(o *ast.ObjectItem) (*api.ConsulGateway, error) {
delete(m, "proxy")
delete(m, "ingress")
delete(m, "terminating")
delete(m, "mesh")

dec, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{
DecodeHook: mapstructure.StringToTimeDurationHookFunc(),
Expand Down Expand Up @@ -274,30 +277,41 @@ func parseGateway(o *ast.ObjectItem) (*api.ConsulGateway, error) {
gateway.Proxy = proxy

// extract and parse the ingress block
io := listVal.Filter("ingress")
if len(io.Items) == 1 {
if io := listVal.Filter("ingress"); len(io.Items) > 0 {
if len(io.Items) > 1 {
return nil, fmt.Errorf("ingress, %s", "multiple ingress stanzas not allowed")
}

ingress, err := parseIngressConfigEntry(io.Items[0])
if err != nil {
return nil, fmt.Errorf("ingress, %v", err)
}
gateway.Ingress = ingress
}

if len(io.Items) > 1 {
return nil, fmt.Errorf("ingress, %s", "multiple ingress stanzas not allowed")
}
if to := listVal.Filter("terminating"); len(to.Items) > 0 {
if len(to.Items) > 1 {
return nil, fmt.Errorf("terminating, %s", "multiple terminating stanzas not allowed")
}

to := listVal.Filter("terminating")
if len(to.Items) == 1 {
terminating, err := parseTerminatingConfigEntry(to.Items[0])
if err != nil {
return nil, fmt.Errorf("terminating, %v", err)
}
gateway.Terminating = terminating
}

if len(to.Items) > 1 {
return nil, fmt.Errorf("terminating, %s", "multiple terminating stanzas not allowed")
if mo := listVal.Filter("mesh"); len(mo.Items) > 0 {
if len(mo.Items) > 1 {
return nil, fmt.Errorf("mesh, %s", "multiple mesh stanzas not allowed")
}

// mesh should have no keys
if err := checkHCLKeys(mo.Items[0].Val, []string{}); err != nil {
return nil, fmt.Errorf("mesh, %s", err)
}

gateway.Mesh = &api.ConsulMeshConfigEntry{}
}

return &gateway, nil
Expand Down Expand Up @@ -891,6 +905,9 @@ func parseUpstream(uo *ast.ObjectItem) (*api.ConsulUpstream, error) {
valid := []string{
"destination_name",
"local_bind_port",
"local_bind_address",
"datacenter",
"mesh_gateway",
}

if err := checkHCLKeys(uo.Val, valid); err != nil {
Expand All @@ -903,6 +920,8 @@ func parseUpstream(uo *ast.ObjectItem) (*api.ConsulUpstream, error) {
return nil, err
}

delete(m, "mesh_gateway")

dec, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{
DecodeHook: mapstructure.StringToTimeDurationHookFunc(),
WeaklyTypedInput: true,
Expand All @@ -916,9 +935,51 @@ func parseUpstream(uo *ast.ObjectItem) (*api.ConsulUpstream, error) {
return nil, err
}

var listVal *ast.ObjectList
if ot, ok := uo.Val.(*ast.ObjectType); ok {
listVal = ot.List
} else {
return nil, fmt.Errorf("'%s': should be an object", upstream.DestinationName)
}

if mgO := listVal.Filter("mesh_gateway"); len(mgO.Items) > 0 {
if len(mgO.Items) > 1 {
return nil, fmt.Errorf("upstream '%s': cannot have more than 1 mesh_gateway", upstream.DestinationName)
}

mgw, err := parseMeshGateway(mgO.Items[0])
if err != nil {
return nil, multierror.Prefix(err, fmt.Sprintf("'%s',", upstream.DestinationName))
}

upstream.MeshGateway = mgw

}
return &upstream, nil
}

func parseMeshGateway(gwo *ast.ObjectItem) (*api.ConsulMeshGateway, error) {
valid := []string{
"mode",
}

if err := checkHCLKeys(gwo.Val, valid); err != nil {
return nil, multierror.Prefix(err, "mesh_gateway ->")
}

var m map[string]interface{}
if err := hcl.DecodeObject(&m, gwo.Val); err != nil {
return nil, err
}

var mgw api.ConsulMeshGateway
if err := mapstructure.WeakDecode(m, &mgw); err != nil {
return nil, err
}

return &mgw, nil
}

func parseChecks(service *api.Service, checkObjs *ast.ObjectList) error {
service.Checks = make([]api.ServiceCheck, len(checkObjs.Items))
for idx, co := range checkObjs.Items {
Expand All @@ -945,6 +1006,8 @@ func parseChecks(service *api.Service, checkObjs *ast.ObjectList) error {
"task",
"success_before_passing",
"failures_before_critical",
"on_update",
"body",
}
if err := checkHCLKeys(co.Val, valid); err != nil {
return multierror.Prefix(err, "check ->")
Expand Down
1 change: 1 addition & 0 deletions jobspec/parse_task.go
Original file line number Diff line number Diff line change
Expand Up @@ -551,6 +551,7 @@ func parseResources(result *api.Resources, list *ast.ObjectList) error {
"memory_max",
"network",
"device",
"cores",
}
if err := checkHCLKeys(listVal, valid); err != nil {
return multierror.Prefix(err, "resources ->")
Expand Down
71 changes: 65 additions & 6 deletions jobspec/parse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,10 +134,12 @@ func TestParse(t *testing.T) {
ExtraKeysHCL: nil,
},
"bar": {
Name: "bar",
Type: "csi",
Source: "bar-vol",
ReadOnly: true,
Name: "bar",
Type: "csi",
Source: "bar-vol",
ReadOnly: true,
AccessMode: "single-mode-writer",
AttachmentMode: "file-system",
MountOptions: &api.CSIMountOptions{
FSType: "ext4",
},
Expand All @@ -152,6 +154,7 @@ func TestParse(t *testing.T) {
"ro",
},
},
PerAlloc: true,
ExtraKeysHCL: nil,
},
},
Expand Down Expand Up @@ -1112,8 +1115,14 @@ func TestParse(t *testing.T) {
LocalServicePort: 8080,
Upstreams: []*api.ConsulUpstream{
{
DestinationName: "other-service",
LocalBindPort: 4567,
DestinationName: "other-service",
LocalBindPort: 4567,
LocalBindAddress: "0.0.0.0",
Datacenter: "dc1",

MeshGateway: &api.ConsulMeshGateway{
Mode: "local",
},
},
},
},
Expand Down Expand Up @@ -1178,6 +1187,7 @@ func TestParse(t *testing.T) {
{
Name: "foo-service",
PortLabel: "http",
OnUpdate: "ignore",
Checks: []api.ServiceCheck{
{
Name: "check-name",
Expand All @@ -1187,6 +1197,8 @@ func TestParse(t *testing.T) {
Timeout: time.Duration(2 * time.Second),
InitialStatus: "passing",
TaskName: "foo",
OnUpdate: "ignore",
Body: "post body",
},
},
},
Expand Down Expand Up @@ -1551,6 +1563,7 @@ func TestParse(t *testing.T) {
"listener2": {Name: "listener2", Address: "10.0.0.2", Port: 8889},
},
EnvoyGatewayNoDefaultBind: true,
EnvoyDNSDiscoveryType: "LOGICAL_DNS",
Config: map[string]interface{}{"foo": "bar"},
},
Terminating: &api.ConsulTerminatingConfigEntry{
Expand All @@ -1571,6 +1584,28 @@ func TestParse(t *testing.T) {
},
false,
},
{
"tg-service-connect-gateway-mesh.hcl",
&api.Job{
ID: stringToPtr("connect_gateway_mesh"),
Name: stringToPtr("connect_gateway_mesh"),
TaskGroups: []*api.TaskGroup{{
Name: stringToPtr("group"),
Services: []*api.Service{{
Name: "mesh-gateway-service",
Connect: &api.ConsulConnect{
Gateway: &api.ConsulGateway{
Proxy: &api.ConsulGatewayProxy{
Config: map[string]interface{}{"foo": "bar"},
},
Mesh: &api.ConsulMeshConfigEntry{},
},
},
}},
}},
},
false,
},
{
"tg-scaling-policy-minimal.hcl",
&api.Job{
Expand Down Expand Up @@ -1664,6 +1699,30 @@ func TestParse(t *testing.T) {
},
false,
},
{
"resources-cores.hcl",
&api.Job{
ID: stringToPtr("cores-test"),
Name: stringToPtr("cores-test"),
TaskGroups: []*api.TaskGroup{
{
Count: intToPtr(5),
Name: stringToPtr("group"),
Tasks: []*api.Task{
{
Name: "task",
Driver: "docker",
Resources: &api.Resources{
Cores: intToPtr(4),
MemoryMB: intToPtr(128),
},
},
},
},
},
},
false,
},
}

for _, tc := range cases {
Expand Down
10 changes: 7 additions & 3 deletions jobspec/test-fixtures/basic.hcl
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,11 @@ job "binstore-storagelocker" {
}

volume "bar" {
type = "csi"
source = "bar-vol"
read_only = true
type = "csi"
source = "bar-vol"
read_only = true
attachment_mode = "file-system"
access_mode = "single-mode-writer"

mount_options {
fs_type = "ext4"
Expand All @@ -92,6 +94,8 @@ job "binstore-storagelocker" {
mount_options {
mount_flags = ["ro"]
}

per_alloc = true
}

restart {
Expand Down
14 changes: 14 additions & 0 deletions jobspec/test-fixtures/resources-cores.hcl
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
job "cores-test" {
group "group" {
count = 5

task "task" {
driver = "docker"

resources {
cores = 4
memory = 128
}
}
}
}
10 changes: 8 additions & 2 deletions jobspec/test-fixtures/tg-network.hcl
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,14 @@ job "foo" {
local_service_port = 8080

upstreams {
destination_name = "other-service"
local_bind_port = 4567
destination_name = "other-service"
local_bind_port = 4567
local_bind_address = "0.0.0.0"
datacenter = "dc1"

mesh_gateway {
mode = "local"
}
}
}
}
Expand Down
7 changes: 5 additions & 2 deletions jobspec/test-fixtures/tg-service-check.hcl
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,9 @@ job "group_service_check_script" {
}

service {
name = "foo-service"
port = "http"
name = "foo-service"
port = "http"
on_update = "ignore"

check {
name = "check-name"
Expand All @@ -23,6 +24,8 @@ job "group_service_check_script" {
timeout = "2s"
initial_status = "passing"
task = "foo"
on_update = "ignore"
body = "post body"
}
}

Expand Down
19 changes: 19 additions & 0 deletions jobspec/test-fixtures/tg-service-connect-gateway-mesh.hcl
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
job "connect_gateway_mesh" {
group "group" {
service {
name = "mesh-gateway-service"

connect {
gateway {
proxy {
config {
foo = "bar"
}
}

mesh {}
}
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ job "connect_gateway_terminating" {
}

envoy_gateway_no_default_bind = true
envoy_dns_discovery_type = "LOGICAL_DNS"

config {
foo = "bar"
Expand Down

0 comments on commit e5a4dde

Please sign in to comment.