Skip to content

Commit

Permalink
Fix Bastion deletion when SSHAccess is disabled (gardener#8421)
Browse files Browse the repository at this point in the history
* allow bastion deletion if SSH access is disabled

* add suggested improvements
  • Loading branch information
AleksandarSavchev authored Sep 7, 2023
1 parent 180951e commit ff79925
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 2 deletions.
3 changes: 2 additions & 1 deletion plugin/pkg/bastion/validator/admission.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/apiserver/pkg/admission"

gardencorehelper "github.com/gardener/gardener/pkg/apis/core/helper"
gardencorev1beta1 "github.com/gardener/gardener/pkg/apis/core/v1beta1"
v1beta1constants "github.com/gardener/gardener/pkg/apis/core/v1beta1/constants"
"github.com/gardener/gardener/pkg/apis/operations"
Expand Down Expand Up @@ -150,7 +151,7 @@ func (v *Bastion) Admit(ctx context.Context, a admission.Attributes, _ admission
}

// ensure shoot SSH access is not disabled
if shoot.Spec.Provider.WorkersSettings != nil && shoot.Spec.Provider.WorkersSettings.SSHAccess != nil && !shoot.Spec.Provider.WorkersSettings.SSHAccess.Enabled {
if bastion.DeletionTimestamp == nil && !gardencorehelper.ShootEnablesSSHAccess(shoot) {
fieldErr := field.Invalid(shootPath, shootName, "ssh access is disabled for worker nodes")
return apierrors.NewInvalid(gk, bastion.Name, field.ErrorList{fieldErr})
}
Expand Down
27 changes: 26 additions & 1 deletion plugin/pkg/bastion/validator/admission_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ const (
bastionName = "foo"
shootName = "foo"
seedName = "foo"
workerName = "foo"
namespace = "garden"
provider = "foo-provider"
region = "foo-region"
Expand Down Expand Up @@ -71,6 +72,11 @@ var _ = Describe("Bastion", func() {
SeedName: pointer.String(seedName),
Provider: gardencore.Provider{
Type: provider,
Workers: []gardencore.Worker{
{
Name: workerName,
},
},
},
Region: region,
},
Expand Down Expand Up @@ -259,7 +265,26 @@ var _ = Describe("Bastion", func() {

oldBastion := bastion.DeepCopy()
oldBastion.ObjectMeta.Finalizers = []string{"foo"}
bastion.ObjectMeta.Finalizers = []string{""}
bastion.ObjectMeta.Finalizers = nil

err := admissionHandler.Admit(context.TODO(), getBastionAttributes(bastion, oldBastion, admission.Update), nil)
Expect(err).To(Succeed())
})

It("should allow the Bastion update on finalizers even if the Shoot's SSH access is disabled", func() {
shoot.Spec.Provider.WorkersSettings = &gardencore.WorkersSettings{
SSHAccess: &gardencore.SSHAccess{Enabled: false},
}
now := metav1.Now()
bastion.DeletionTimestamp = &now

coreClient.AddReactor("get", "shoots", func(action testing.Action) (bool, runtime.Object, error) {
return true, shoot, nil
})

oldBastion := bastion.DeepCopy()
oldBastion.ObjectMeta.Finalizers = []string{"foo"}
bastion.ObjectMeta.Finalizers = nil

err := admissionHandler.Admit(context.TODO(), getBastionAttributes(bastion, oldBastion, admission.Update), nil)
Expect(err).To(Succeed())
Expand Down

0 comments on commit ff79925

Please sign in to comment.