Skip to content

release-22.1: sql: disallow tenants from setting zone configs by default#79160

Merged
arulajmani merged 1 commit intorelease-22.1from
blathers/backport-release-22.1-79097
Apr 4, 2022
Merged

release-22.1: sql: disallow tenants from setting zone configs by default#79160
arulajmani merged 1 commit intorelease-22.1from
blathers/backport-release-22.1-79097

Conversation

@blathers-crl
Copy link
Copy Markdown

@blathers-crl blathers-crl bot commented Mar 31, 2022

Backport 1/1 commits from #79097 on behalf of @arulajmani.

/cc @cockroachdb/release


Previously, sql.zone_configs.allow_for_secondary_tenant.enabled was a
regular old cluster setting that tenants could control. Now that we have
a notion of tenant read-only setting, this feels like a great candidate
for it. In addition to making this switch, this patch also changes this
setting's default value to be false. See the linked issue for the
motivation around why.

The change to switch this setting to be tenant read-only necessitates
changes in how we run logic tests. This is because logic tests that
run as secondary tenants can no longer change this cluster setting
in the test file. We introduce a new
tenant-cluster-setting-override-opt logic test directive with an
option for allow-zone-configs-for-secondary-tenants to get this
working. When configured, the host alters this cluster setting
during test setup. This can later be extended for other read-only
settings in the future.

References #77344

Release note (sql change): Changes the default value of
sql.zone_configs.allow_for_secondary_tenant.enabled to be false.
Moreover, this setting is no longer settable by secondary tenants.
Instead, it's now a tenant read-only cluster setting.


Release justification: "low risk change to new functionality"

@blathers-crl blathers-crl bot requested a review from a team as a code owner March 31, 2022 18:52
@blathers-crl blathers-crl bot force-pushed the blathers/backport-release-22.1-79097 branch from feec34f to 2de953e Compare March 31, 2022 18:52
@blathers-crl
Copy link
Copy Markdown
Author

blathers-crl bot commented Mar 31, 2022

Thanks for opening a backport.

Please check the backport criteria before merging:

  • Patches should only be created for serious issues or test-only changes.
  • Patches should not break backwards-compatibility.
  • Patches should change as little code as possible.
  • Patches should not change on-disk formats or node communication protocols.
  • Patches should not add new functionality.
  • Patches must not add, edit, or otherwise modify cluster versions; or add version gates.
If some of the basic criteria cannot be satisfied, ensure that the exceptional criteria are satisfied within.
  • 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.
  • The PM and TL on the team that owns the changed code have signed off that the change obeys the above rules.

Add a brief release justification to the body of your PR to justify this backport.

Some other things to consider:

  • What did we do to ensure that a user that doesn’t know & care about this backport, has no idea that it happened?
  • Will this work in a cluster of mixed patch versions? Did we test that?
  • If a user upgrades a patch version, uses this feature, and then downgrades, what happens?

@blathers-crl blathers-crl bot added blathers-backport This is a backport that Blathers created automatically. O-robot Originated from a bot. labels Mar 31, 2022
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

Previously, `sql.zone_configs.allow_for_secondary_tenant.enabled` was a
regular old cluster setting that tenants could control. Now that we have
a notion of tenant read-only setting, this feels like a great candidate
for it. In addition to making this switch, this patch also changes this
setting's default value to be false. See the linked issue for the
motivation around why.

The change to switch this setting to be tenant read-only necessitates
changes in how we run logic tests. This is because logic tests that
run as secondary tenants can no longer change this cluster setting
in the test file. We introduce a new
`tenant-cluster-setting-override-opt` logic test directive with an
option for `allow-zone-configs-for-secondary-tenants` to get this
working. When configured, the host alters this cluster setting
during test setup. This can later be extended for other read-only
settings in the future.

References #77344

Release note (sql change): Changes the default value of
`sql.zone_configs.allow_for_secondary_tenant.enabled` to be false.
 Moreover, this setting is no longer settable by secondary tenants.
Instead, it's now a tenant read-only cluster setting.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

blathers-backport This is a backport that Blathers created automatically. O-robot Originated from a bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants