Add support for SPIFFE bundle sequence number #4061
Conversation
maxlambrecht
requested review from
evan2645,
amartinezfayo,
azdagron,
MarcosDY and
rturner3
as code owners
| @@ -2494,6 +2500,7 @@ func TestBatchSetFederatedBundle(t *testing.T) { | |||
| updatedBundle := makeValidBundle(t, federatedTrustDomain) | |||
| // Change the refresh hint | |||
| updatedBundle.RefreshHint = 120 | |||
| updatedBundle.SequenceNumber = 42 | |||
There was a problem hiding this comment.
are you allowing to set a sequence number on creating and when updating a bundle?
I think we must prevent that to happens
There was a problem hiding this comment.
Ok,
here is the situation,
1- In case of "server trust domain" we must not allow update sequence number, I think that is covered in case since we allow to call create or update only on federated bundles.
2- in case of federated bundles, we must allow a regular CRUD where third parties can choose any Sequence they want
3- we must allow to set the sequence on federated bundles on creation (to keep the same sequence that is on federated server database)
4- we must prevent to set something different than 0 when creating "server" bundle
So looks like we are safe, but we'll need to double check all
| @@ -1088,6 +1093,7 @@ func pruneBundle(tx *gorm.DB, trustDomainID string, expiry time.Time, log logrus | |||
|
|
|||
| // Update only if bundle was modified | |||
| if changed { | |||
| newBundle.SequenceNumber++ | |||
There was a problem hiding this comment.
newBundle will always come with SequenceNumber = 0, so this will reset the SequenceNumber to 1 when pruning. We should increment the number from the current bundle.
There was a problem hiding this comment.
Good catch! I fixed it and updated the prune test to use sequence numbers other than 0 and 1 to catch this.
Signed-off-by: Max Lambrecht <[email protected]>
Signed-off-by: Max Lambrecht <[email protected]>
maxlambrecht
force-pushed
the
add-support-for-bundle-sequence-number
branch
from
b9646a0 to
3c8c9d5
Compare
* Add support for SPIFFE bundle sequence number Signed-off-by: Max Lambrecht <[email protected]> Signed-off-by: Dmitry Gorochovsky <[email protected]>
Pull Request check list
Affected functionality
Adds supports for SPIFFE Bundle sequence number.
Description of change
A new field
sequence_numberwas added to theBundletype defined incommon.proto.The
datastoreAppend and Prune methods increment the sequence number when the bundle was changed.This sequence number is saved to the DB.
The
sequence_numberis now set in the Bundle services responses.Which issue this PR fixes
Fixes #1495