cockroach icon indicating copy to clipboard operation
cockroach copied to clipboard

release-24.1: server/license: store cluster init time in the KV system keyspace

Open spilchen opened this issue 1 year ago • 3 comments

Backport 1/1 commits from #129634.

/cc @cockroachdb/release


We need to store cluster-level metadata for license policy enforcement. This metadata must be protected from modification, so it cannot be stored in any system table. This PR introduces a new field in the system keyspace to store the cluster's initialization start time, which will be used in a follow-up PR to track the grace period for clusters without a license. The metadata needs to be stored at the cluster level to align with the license's scope.

A new struct, License.Enforcer, is responsible for writing and caching the new timestamp. This struct will be extended in future PRs to check for throttling in clusters that violate our license policies.

This update will be backported to versions 23.1, 23.2, 24.1, and 24.2.

Epic: CRDB-39988 Informs: CRDB-40064 Release note: None Release justification: This work is part of the CockroachDB core deprecation.

spilchen avatar Aug 28 '24 12:08 spilchen

Thanks for opening a backport.

Please check the backport criteria before merging:

  • [ ] Backports should only be created for serious issues or test-only changes.
  • [ ] Backports should not break backwards-compatibility.
  • [ ] Backports should change as little code as possible.
  • [ ] Backports should not change on-disk formats or node communication protocols.
  • [ ] Backports should not add new functionality (except as defined here).
  • [ ] Backports must not add, edit, or otherwise modify cluster versions; or add version gates.
  • [ ] All backports must be reviewed by the owning areas TL. For more information as to how that review should be conducted, please consult the backport policy.
If your backport adds new functionality, please ensure that the following additional criteria are satisfied:
  • [ ] There is a high priority need for the functionality that cannot wait until the next release and is difficult to address in another way.
  • [ ] The new functionality is additive-only and only runs for clusters which have specifically “opted in” to it (e.g. by a cluster setting).
  • [ ] New code is protected by a conditional check that is trivial to verify and ensures that it only runs for opt-in clusters. State changes must be further protected such that nodes running old binaries will not be negatively impacted by the new state (with a mixed version test added).
  • [ ] The PM and TL on the team that owns the changed code have signed off that the change obeys the above rules.
  • [ ] Your backport must be accompanied by a post to the appropriate Slack channel (#db-backports-point-releases or #db-backports-XX-X-release) for awareness and discussion.

Also, please add a brief release justification to the body of your PR to justify this backport.

blathers-crl[bot] avatar Aug 28 '24 12:08 blathers-crl[bot]

This change is Reviewable

cockroach-teamcity avatar Aug 28 '24 12:08 cockroach-teamcity

Good point. Are you suggesting we wait to merge this until we have more functionality in place or until we get closer to the October 24.1 release?

spilchen avatar Aug 28 '24 19:08 spilchen

As discussed, we decided to make the KV layer write opt-in for now and limit it to testing (see #129898).

@rafiss I know it's not the usual approach, but do you think it makes sense to cherry-pick that commit into this PR? That way, both changes are handled together. Alternatively, I could keep them separate and still get them in on the same day. It would have to be next week, though, as I’m off tomorrow.

spilchen avatar Aug 30 '24 01:08 spilchen

do you think it makes sense to cherry-pick that commit into this PR?

yes, let's do it this way. thanks!

rafiss avatar Aug 30 '24 17:08 rafiss