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

Use non-experimental version of encryption provider config flag in 1.13+ #7900

Merged

Conversation

zacblazic
Copy link
Contributor

As of kubernetes 1.13, the --encryption-provider-config flag should be used instead of the variant that provided alpha support for the feature.

From https://v1-13.docs.kubernetes.io/docs/setup/release/notes/#deprecations:

The --experimental-encryption-provider-config flag is deprecated in favor of --encryption-provider-config. The old flag is accepted with a warning but will be removed in 1.14.

The alpha version of encryption at rest used the following flag:
`--experimental-encryption-provider-config`. As of kubernetes 1.13,
`--encryption-provider-config` should be used instead.
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Nov 8, 2019
@k8s-ci-robot
Copy link
Contributor

Hi @zacblazic. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Nov 8, 2019
@rifelpet
Copy link
Member

/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Nov 22, 2019
@joshbranham
Copy link
Contributor

Looks like we are still discussing a formal deprecation policy, but wondering what the thought here is on the old flag. I agree that it should be a new field so it's clear what is happening for 1.13+, but maybe we should also be alerting users via kops update cluster if they are using the --experimental-encryption-provider-config flag still?

@johngmyers
Copy link
Member

/test pull-kops-verify-staticcheck

@hakman
Copy link
Member

hakman commented Jan 5, 2020

@joshbranham from the user's point of view this change is transparent. The config option remains the same: Cluster.Spec.EncryptionConfig, so no need for a warning.

I see in code that the "experimental" flag was not yet removed, so at least nothing is broken.

@justinsb
Copy link
Member

justinsb commented Jan 5, 2020

This looks good to me; we maybe should only introduce it to newer k8s versions (so we don't change configuration for older kubernetes versions), but they are aliases to each other, and they are so similar I don't think they are going to cause any confusion.

We could (separately) warn if someone is explicitly specifying experimentalEncryptionProviderConfig - but as a typical user won't do this I don't think we need to - we don't make compatibility guarantees for the flag-level settings.

/approve
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 5, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: justinsb, zacblazic

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 5, 2020
@k8s-ci-robot k8s-ci-robot merged commit 95f4f83 into kubernetes:master Jan 5, 2020
@k8s-ci-robot k8s-ci-robot added this to the v1.18 milestone Jan 5, 2020
@Nilubkal
Copy link

Will this change be ported into v1.17 too ?

k8s-ci-robot added a commit that referenced this pull request Apr 23, 2020
…-origin-release-1.17

Automated cherry pick of #7900: Add encryptionProviderConfig field
k8s-ci-robot added a commit that referenced this pull request Apr 23, 2020
…-origin-release-1.16

Automated cherry pick of #7900: Add encryptionProviderConfig field
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants