Using v1/agent/check/register requires node:write when registering a check for a specific service.

[mkeeler]

Overview of the Issue

Consul is requiring node:write on the agent node for check registration even though a service level check is being registered.

Code Deep Dive

Decoding of Request

First we decode the structs.CheckDefinition

consul/agent/agent_endpoint.go

Lines 556 to 565 in f2213f6

	var args structs.CheckDefinition
	// Fixup the type decode of TTL or Interval.
	decodeCB := func(raw interface{}) error {
	return FixupCheckType(raw)
	}
	if err := decodeBody(req, &args, decodeCB); err != nil {
	resp.WriteHeader(http.StatusBadRequest)
	fmt.Fprintf(resp, "Request decode failed: %v", err)
	return nil, nil
	}

The check definition contains a service id in it but not a service name.

Type Conversion

Then we convert the structs.CheckDefinition into a structs.HealthCheck here:

consul/agent/agent_endpoint.go

Line 581 in f2213f6

health := args.HealthCheck(s.agent.config.NodeName)

This is implemented here:

consul/agent/structs/check_definition.go

Lines 42 to 58 in f2213f6

	func (c *CheckDefinition) HealthCheck(node string) *HealthCheck {
	health := &HealthCheck{
	Node: node,
	CheckID: c.ID,
	Name: c.Name,
	Status: api.HealthCritical,
	Notes: c.Notes,
	ServiceID: c.ServiceID,
	}
	if c.Status != "" {
	health.Status = c.Status
	}
	if health.CheckID == "" && health.Name != "" {
	health.CheckID = types.CheckID(health.Name)
	}
	return health
	}

Basically it copies the ServiceID in the CheckDefinition to the ServiceID in the HealthCheck but leaves the ServiceName in the HealthCheck untouched.

Check ACLs

Back in the AgentRegisterCheck function we need to verify that the entity making the check registration request has adequate privileges so we invoke vetCheckRegister here:

consul/agent/agent_endpoint.go

Lines 592 to 597 in f2213f6

	// Get the provided token, if any, and vet against any ACL policies.
	var token string
	s.parseToken(req, &token)
	if err := s.agent.vetCheckRegister(token, health); err != nil {
	return nil, err
	}

Then within vetCheckRegister we check specifically for the ServiceName on the HealthCheck here:

consul/agent/acl.go

Lines 151 to 160 in f2213f6

	// Vet the check itself.
	if len(check.ServiceName) > 0 {
	if !rule.ServiceWrite(check.ServiceName, nil) {
	return acl.ErrPermissionDenied
	}
	} else {
	if !rule.NodeWrite(a.config.NodeName, nil) {
	return acl.ErrPermissionDenied
	}
	}

End Result

Because the ServiceName was never set, the ServiceID is ignored and we are treating this as a node level check and requiring node:write instead of treating it as a service level check and only requiring service:write.

Possible solution

Before calling vetCheckRegister we could look up the ServiceID within the local state and fixup the ServiceName on the HealthCheck structure.