Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

release-22.1: roachtest: declare encryption support as part of TestSpec #81588

Merged
merged 1 commit into from
May 24, 2022

Conversation

renatolabs
Copy link
Contributor

@renatolabs renatolabs commented May 20, 2022

Backport 1/1 commits from #81483.

/cc @cockroachdb/release


This commit introduces the EncryptionSupport field to TestSpec. By
setting this field, tests can declare whether they support being run
in a cluster with encryption enabled. Three options are available:

  • EncryptionAllowed: test can run with or without encryption.
  • EncryptionRequired: test can only run if encryption is enabled.
  • EncryptionDisabled: test can only run if encryption is disabled (default).

This replaces the EncryptAtRandom field added earlier, which only
allowed tests to opt-in to random encryption.

In addition to this change, the values accepted by the --encrypt
flag in roachtest also changes. The only two valid values are:

  • --encrypt=auto (default). Tests that support encryption
    (EncryptionAllowed) will run with encryption enabled.
  • --encrypt=random (CI). Tests that support encryption
    (EncryptionAllowed) will run on a cluster which may or may
    not have encryption enabled.

The EncryptionSupport field should be the only way tests indicate
their relationship with encryption. The DontEncrypt field was
removed (it was not being used anymore at the time of this commit),
and all tests that set EncryptedStores directly were updated to use
EncryptionSupport accordingly.

Resolves: #79265.

Release note: None


Release justification: fix roachtest.

This commit introduces the `EncryptionSupport` field to `TestSpec`. By
setting this field, tests can declare whether they support being run
in a cluster with encryption enabled. Three options are available:

- EncryptionAlwaysDisabled: test can only run if encryption is
  disabled (default).
- EncryptionAlwaysEnabled: test can only run if encryption is enabled.
- EncryptionMetamorphic: tests can opt-in to metamorphic encryption;
  the probability of these tests running with encryption enabled is
  controlled by a command flag.

This replaces the `EncryptAtRandom` field added earlier, which only
allowed tests to opt-in to random encryption.

In addition to this change, the `--encrypt` flag is superseded by a
new `--metamorphic-encryption-probability` flag. As the name implies,
this flag controls the probability of enabling encryption for tests
that opted-in to metamorphic encryption. By default, this value is 1,
meaning that tests with EncryptionMetamorphic set will always run with
encryption enabled. To run these tests locally with encryption
disabled, developers can call roachtest passing
--metamorphic-encryption-probability=0.

The `EncryptionSupport` field should be the only way tests indicate
their relationship with encryption. The `DontEncrypt` field was
removed (it was not being used anymore at the time of this commit),
and all tests that set `EncryptedStores` directly were updated to use
`EncryptionSupport` accordingly.

Resolves: cockroachdb#79265.

Release note: None
@renatolabs renatolabs requested review from a team as code owners May 20, 2022 19:59
@renatolabs renatolabs requested review from otan and removed request for a team May 20, 2022 19:59
@blathers-crl
Copy link

blathers-crl bot commented May 20, 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?

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@srosenberg srosenberg self-requested a review May 20, 2022 21:07
@otan otan removed their request for review May 23, 2022 17:41
@renatolabs renatolabs merged commit 2ed2491 into cockroachdb:release-22.1 May 24, 2022
@renatolabs renatolabs deleted the backport22.1-81483 branch May 24, 2022 12:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants