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

Prepare for the removal of the enableBasicAuthentication field #7534

Merged
merged 8 commits into from
Feb 27, 2023

Conversation

ialidzhikov
Copy link
Member

@ialidzhikov ialidzhikov commented Feb 21, 2023

How to categorize this PR?

/kind cleanup

What this PR does / why we need it:
The enableBasicAuthentication field does no longer make sense after #6987. In #6987 we removed support for Shoots with K8s < 1.20. The basic authentication was removed in K8s 1.19.x. Hence, it is not possible to enable basic auth anymore for a Shoot that is within the support K8s versions of Gardener currently.

This PR is a preparation for the removal of the enableBasicAuthentication field:

  • It makes the enableBasicAuthentication field no-op (removes validation, removes defaulting) - the field is not used anymore.
  • It sets the enableBasicAuthentication field always to nil. So far the field was always defaulted to false, if it was not specified in the field.
  • It overwrites the .spec.kubernetes.addons.kubernetesDashboard.authenticationMode to token, if it is set to basic.

Which issue(s) this PR fixes:
Part of #6911

Special notes for your reviewer:
N/A

Release note:

The `.spec.kubernetes.kubeAPIServer.enableBasicAuthentication` field is now no-op - the `gardener-apiserver` no longer defaults this field and no longer validates it. The field will be set always to `nil` on CREATE/UPDATE request.
End users specifying this field should no longer specify it. The field will be removed in a future version of Gardener.

@gardener-prow gardener-prow bot added kind/cleanup Something that is not needed anymore and can be cleaned up cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. labels Feb 21, 2023
@gardener-prow gardener-prow bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Feb 21, 2023
@gardener-prow gardener-prow bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 22, 2023
@gardener-prow gardener-prow bot added cla: no Indicates the PR's author has not signed the cla-assistant.io CLA. cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. cla: no Indicates the PR's author has not signed the cla-assistant.io CLA. labels Feb 22, 2023
@rfranzke
Copy link
Member

/assign

pkg/apis/core/v1alpha1/defaults.go Show resolved Hide resolved
pkg/registry/core/shoot/strategy_test.go Outdated Show resolved Hide resolved
pkg/registry/core/shoot/strategy.go Outdated Show resolved Hide resolved
@gardener-prow gardener-prow bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 23, 2023
ialidzhikov and others added 8 commits February 23, 2023 15:53
Co-authored-by: dimitar-kostadinov <dimitar.kostadinov@sap.com>
Co-authored-by: dimitar-kostadinov <dimitar.kostadinov@sap.com>
…pareForCreate and PrepareForUpdate

Co-authored-by: dimitar-kostadinov <dimitar.kostadinov@sap.com>
The field is now no-op.

Co-authored-by: dimitar-kostadinov <dimitar.kostadinov@sap.com>
Co-authored-by: dimitar-kostadinov <dimitar.kostadinov@sap.com>
…anifest used by test machinery

Co-authored-by: dimitar-kostadinov <dimitar.kostadinov@sap.com>
…value is `basic`

Co-authored-by: dimitar-kostadinov <dimitar.kostadinov@sap.com>
@gardener-prow gardener-prow bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 23, 2023
@gardener-prow
Copy link
Contributor

gardener-prow bot commented Feb 23, 2023

@ialidzhikov: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
pull-gardener-apidiff a14ff33 link false /test pull-gardener-apidiff
pull-gardener-e2e-kind-ha-multi-zone-upgrade a14ff33 link false /test pull-gardener-e2e-kind-ha-multi-zone-upgrade

Full PR test history. Your PR dashboard. Command help for this repository.
Please help us cut down on flakes by linking this test failure to an open flake report or filing a new flake report if you can't find an existing one. Also see our testing guideline for how to avoid and hunt flakes.

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. I understand the commands that are listed here.

@rfranzke
Copy link
Member

/lgtm
/approve
/retest-required

@gardener-prow gardener-prow bot added the lgtm Indicates that a PR is ready to be merged. label Feb 24, 2023
@gardener-prow
Copy link
Contributor

gardener-prow bot commented Feb 24, 2023

LGTM label has been added.

Git tree hash: 3ffb27ec8b54336f3ab5f1aa9d647a8b97255a28

@gardener-prow
Copy link
Contributor

gardener-prow bot commented Feb 24, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rfranzke

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

@gardener-prow gardener-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 24, 2023
@gardener-prow gardener-prow bot merged commit e061ff9 into gardener:master Feb 27, 2023
@ialidzhikov ialidzhikov deleted the cleanup/basic-auth-field branch March 1, 2023 09:16
andrerun pushed a commit to andrerun/gardener that referenced this pull request Jul 6, 2023
…dener#7534)

* Remove the validation for the `enableBasicAuthentication` field

Co-authored-by: dimitar-kostadinov <dimitar.kostadinov@sap.com>

* Remove the defaulting for the `enableBasicAuthentication` field

Co-authored-by: dimitar-kostadinov <dimitar.kostadinov@sap.com>

* Set the `enableBasicAuthentication` field to nil in shootStrategy PrepareForCreate and PrepareForUpdate

Co-authored-by: dimitar-kostadinov <dimitar.kostadinov@sap.com>

* Update the doc string of the `enableBasicAuthentication` field

The field is now no-op.

Co-authored-by: dimitar-kostadinov <dimitar.kostadinov@sap.com>

* Remove the unused helper function

Co-authored-by: dimitar-kostadinov <dimitar.kostadinov@sap.com>

* Remove the `enableBasicAuthentication` field from the default Shoot manifest used by test machinery

Co-authored-by: dimitar-kostadinov <dimitar.kostadinov@sap.com>

* Set `kubernetesDashboard.authenticationMode` field to `token` if its value is `basic`

Co-authored-by: dimitar-kostadinov <dimitar.kostadinov@sap.com>

* Drop the `KubernetesDashboardAuthModeBasic` const

---------

Co-authored-by: dimitar-kostadinov <dimitar.kostadinov@sap.com>
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. cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. kind/cleanup Something that is not needed anymore and can be cleaned up lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants