ConnectCALeafRequest should factor in DNSSAN and IPSAN into its cache key

[mkeeler]

consul/agent/cache-types/connect_ca_leaf.go

Lines 640 to 672 in e5d6273

	type ConnectCALeafRequest struct {
	Token string
	Datacenter string
	Service string // Service name, not ID
	Agent string // Agent name, not ID
	DNSSAN []string
	IPSAN []net.IP
	MinQueryIndex uint64
	MaxQueryTime time.Duration
	structs.EnterpriseMeta
	}
	func (r *ConnectCALeafRequest) Key() string {
	if len(r.Agent) > 0 {
	return fmt.Sprintf("agent:%s", r.Agent)
	}
	r.EnterpriseMeta.Normalize()
	v, err := hashstructure.Hash([]interface{}{
	r.Service,
	r.EnterpriseMeta,
	}, nil)
	if err == nil {
	return fmt.Sprintf("service:%d", v)
	}
	// If there is an error, we don't set the key. A blank key forces
	// no cache for this request so the request is forwarded directly
	// to the server.
	return ""
	}

When computing the cache key for this type it should also be including the IPSAN and DNSSAN fields in the hash.

Currently there is no way for user provided input to set these fields and they are only used for auto-encrypt. Therefore this shouldn't be affecting anyone today. If we were to allow for setting these via an API or configuration then it could be problematic.

Regardless the "correct" behavior for this cache type should be to include those two slices in the hash structure call.

[sashaaKr]

@mkeeler Hey,
I would like to give a try to take this one.

So if I'm understand the scope of this change in func (r *ConnectCALeafRequest) Key() string function, am I right?

[mkeeler]

@sashaaKr Yes that is correct. There is a hashstructure call in that method that should probably include both of those other two fields. Besides that method we would need to add a new test (probably in agent/cache-types/connect_ca_leaf_test.go) to verify that a leaf request for the same service with different SANs outputs different values for the cache key.

One other thing to note here is that the cache key doesn't take the token or datacenter into account as that is information that the cache implementation always pulls out itself.

[sashaaKr]

@mkeeler great, I think I can handle it

[sashaaKr]

@mkeeler hope this is ok:
#7597

[crhino]

Fixed by #7597