x509 cache hint for registration entries

[ValFadeev]

This is a draft PR aiming to implement some of the suggestions discussed in #6129. It adds a new field to RegisteredEntry represented by an opaque protobuf-encoded message. The message is meant to contain flags and other fields guiding the agent's behavior with respect to pre-fetching and caching of X509 SVIDs. The intention is to reduce unnecessary work performed by the agent, in case the client is unlikely to ever ask for an X509 SVID and prefers JWT instead. For now only one boolean flag JWTOnly is introduced. Other fields may be added later to fine-tune the logic and cover various edge-cases.
The bulk of this draft is to define the new field in the message definition, update the datastore and have the existing tests pass. The actual usage of the field in fetchEntries is purely indicative and requires further consideration.

Pull Request check list

• Commit conforms to CONTRIBUTING.md?
• Proper tests/regressions included?
• Documentation updated?

Affected functionality

• adds a new field to RegisteredEntry and the underlying datastore table
• allows flagging registration entries as preferring JWT SVID over X509
  Description of change

Which issue this PR fixes

Fixes #6129

[amartinezfayo]

Hi @ValFadeev, are you planning to go back to this PR?

[ValFadeev]

Hi @amartinezfayo, apologies for the delay. I had a few conflicting priorities lately, but I do plan to pick this back up over the coming week.

[ValFadeev]

@amartinezfayo offering this for another review. I am not sure I implemented the semantics of an optional field the right way when loading from row. I also understand this change may have to be split in two, with the schema migration going first. But hopefully, this gives an outline of the functionality, as it was last described.

[sorindumitru]

This would mean that the entry is not added to the cache, which means that no workload would be able to request a SVID for it. I think the flag should end up in the lru cache where it can inform if the entry needs to prefetched or not but still allow a workload to request an x509-svid if it so desires.

I think syncSVIDsWithSubscribers might be the place to do this.

[sorindumitru]

Thanks @ValFadeev, this is shaping up nicely. Only one small comment.

[sorindumitru]

Looks like we set the AdditionalAttributes twice. Once here and once in a few lines down.

[sorindumitru]

Hi @ValFadeev , sorry for the delays here. We've merged the API changes, would you mind repointing the dependency to the github.com/spiffe/spire-api-sdk@next?

[sorindumitru]

I think this makes it impossible to remove the flag once set. We need a way to determine if the flag was set or not. Maybe we can use BoolFunc so that we know if it was set or not? We could even have a more general bool "setAdditionalAttributes" which is set to true if the BoolFunc is called?

[ValFadeev]

That is not what I observe - this is run on a clean build of the server:

./bin/spire-server entry show -entryID prefetch-disabled -socketPath=/tmp/spire-server/private/api.sock
Found 1 entry
Entry ID                : prefetch-disabled
SPIFFE ID               : spiffe://example.org/workload-with-prefetch-disabled
Parent ID               : spiffe://example.org/cluster/test
Revision                : 6
X509-SVID TTL           : default
JWT-SVID TTL            : default
Selector                : unix:uid:1004
DisableX509SvidPrefetch : true

Now if an update performed without the flag set it is cleared in the entry:

./bin/spire-server entry update -entryID prefetch-disabled -selector "unix:uid:1004" -parentID spiffe://example.org/cluster/test -spiffeID spiffe://example.org/workload-with-prefetch-disabled -socketPath=/tmp/spire-server/private/api.sock
Entry ID                : prefetch-disabled
SPIFFE ID               : spiffe://example.org/workload-with-prefetch-disabled
Parent ID               : spiffe://example.org/cluster/test
Revision                : 7
X509-SVID TTL           : default
JWT-SVID TTL            : default
Selector                : unix:uid:1004

and set again:

./bin/spire-server entry update -entryID prefetch-disabled -selector "unix:uid:1004" -parentID spiffe://example.org/cluster/test -spiffeID spiffe://example.org/workload-with-prefetch-disabled -disableX509SVIDPrefetch -socketPath=/tmp/spire-server/private/api.sock
Entry ID                : prefetch-disabled
SPIFFE ID               : spiffe://example.org/workload-with-prefetch-disabled
Parent ID               : spiffe://example.org/cluster/test
Revision                : 8
X509-SVID TTL           : default
JWT-SVID TTL            : default
Selector                : unix:uid:1004
DisableX509SvidPrefetch : true

Tha said, it could indeed be useful to introduce an overarching bool, such as "setAdditionalAttributes", to indicate whether any one of the additional attributes has been provided, to set it up for generalisation. I suppose the flag definition could similar to the below one (for every new additional attribute implemented"

	f.BoolFunc("disableX509SVIDPrefetch", "A boolean value that, when set, disables prefetching X509 SVID for this entry", 
	func(_ string) error {
		c.additionalAttributesSet = true
		c.disableX509SVIDPrefetch = true
		return nil
	})

So that the entry field would be populated like this

	if c.additionalAttributesSet {
		e.AdditionalAttributes = &types.Entry_AdditionalAttributes{}
	}

	if c.disableX509SVIDPrefetch {
		e.AdditionalAttributes.DisableX509SvidPrefetch = true
	}

If this makes sense, I can make the adjustment.

[sorindumitru]

Maybe we could also enforce this in the data store. It would at least make for better error messages, if nothing else.

[sorindumitru]

Since version 24 was release in 1.14.3/1.14.4, we no longer need to fallthrough. Sorry about that, should have realised that would be the case when I asked you to do this.

[sorindumitru]

LGTM,thanks @ValFadeev !

[kfox1111]

I'm curious. would the AdditionalAttributes support in this pr make it easier to add #6069