Skip to content

Commit

Permalink
Do not remove legacy fields when .spec.accessRestrictions is removed (
Browse files Browse the repository at this point in the history 
#10885)

Co-authored-by: rfranzke <[email protected]>
  • Loading branch information
@gardener-ci-robot @rfranzke
gardener-ci-robot and rfranzke authored Nov 20, 2024
1 parent 8dc6e41 commit 4e3b0ac
Show file tree
Hide file tree
Showing 6 changed files with 20 additions and 51 deletions.
11 changes: 0 additions & 11 deletions pkg/apiserver/registry/core/cloudprofile/strategy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,12 +129,6 @@ func syncLegacyAccessRestrictionLabelWithNewField(cloudProfile *core.CloudProfil
}

func syncLegacyAccessRestrictionLabelWithNewFieldOnUpdate(cloudProfile, oldCloudProfile *core.CloudProfile) {
hasAccessRestriction := func(accessRestrictions []core.AccessRestriction, name string) bool {
return slices.ContainsFunc(accessRestrictions, func(accessRestriction core.AccessRestriction) bool {
return accessRestriction.Name == name
})
}

removeAccessRestriction := func(accessRestrictions []core.AccessRestriction, name string) []core.AccessRestriction {
var updatedAccessRestrictions []core.AccessRestriction
for _, accessRestriction := range accessRestrictions {
Expand All @@ -157,10 +151,5 @@ func syncLegacyAccessRestrictionLabelWithNewFieldOnUpdate(cloudProfile, oldCloud
cloudProfile.Spec.Regions[i].Labels["seed.gardener.cloud/eu-access"] != "true" {
cloudProfile.Spec.Regions[i].AccessRestrictions = removeAccessRestriction(cloudProfile.Spec.Regions[i].AccessRestrictions, "eu-access-only")
}

if hasAccessRestriction(oldRegion.AccessRestrictions, "eu-access-only") &&
!hasAccessRestriction(cloudProfile.Spec.Regions[i].AccessRestrictions, "eu-access-only") {
delete(cloudProfile.Spec.Regions[i].Labels, "seed.gardener.cloud/eu-access")
}
}
}
4 changes: 2 additions & 2 deletions pkg/apiserver/registry/core/cloudprofile/strategy_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,15 +171,15 @@ var _ = Describe("Strategy", func() {
Expect(newCloudProfile.Spec.Regions[0].Labels).To(BeEmpty())
})

It("should remove the label when the access restriction is dropped", func() {
It("should not remove the label when the access restriction is dropped", func() {
oldCloudProfile.Spec.Regions[0].Labels = map[string]string{"seed.gardener.cloud/eu-access": "true"}
oldCloudProfile.Spec.Regions[0].AccessRestrictions = []core.AccessRestriction{{Name: "eu-access-only"}}
newCloudProfile.Spec.Regions[0].Labels = map[string]string{"seed.gardener.cloud/eu-access": "true"}

cloudprofileregistry.Strategy.PrepareForUpdate(context.Background(), newCloudProfile, oldCloudProfile)

Expect(newCloudProfile.Spec.Regions[0].AccessRestrictions).To(BeEmpty())
Expect(newCloudProfile.Spec.Regions[0].Labels).To(BeEmpty())
Expect(newCloudProfile.Spec.Regions[0].Labels).To(Equal(map[string]string{"seed.gardener.cloud/eu-access": "true"}))
})
})

Expand Down
11 changes: 0 additions & 11 deletions pkg/apiserver/registry/core/seed/strategy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -182,12 +182,6 @@ func syncLegacyAccessRestrictionLabelWithNewField(seed *core.Seed) {
}

func syncLegacyAccessRestrictionLabelWithNewFieldOnUpdate(seed, oldSeed *core.Seed) {
hasAccessRestriction := func(accessRestrictions []core.AccessRestriction, name string) bool {
return slices.ContainsFunc(accessRestrictions, func(accessRestriction core.AccessRestriction) bool {
return accessRestriction.Name == name
})
}

removeAccessRestriction := func(accessRestrictions []core.AccessRestriction, name string) []core.AccessRestriction {
var updatedAccessRestrictions []core.AccessRestriction
for _, accessRestriction := range accessRestrictions {
Expand All @@ -202,9 +196,4 @@ func syncLegacyAccessRestrictionLabelWithNewFieldOnUpdate(seed, oldSeed *core.Se
seed.Labels["seed.gardener.cloud/eu-access"] != "true" {
seed.Spec.AccessRestrictions = removeAccessRestriction(seed.Spec.AccessRestrictions, "eu-access-only")
}

if hasAccessRestriction(oldSeed.Spec.AccessRestrictions, "eu-access-only") &&
!hasAccessRestriction(seed.Spec.AccessRestrictions, "eu-access-only") {
delete(seed.Labels, "seed.gardener.cloud/eu-access")
}
}
4 changes: 2 additions & 2 deletions pkg/apiserver/registry/core/seed/strategy_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,15 +129,15 @@ var _ = Describe("Strategy", func() {
Expect(newSeed.Labels).To(BeEmpty())
})

It("should remove the label when the access restriction is dropped", func() {
It("should not remove the label when the access restriction is dropped", func() {
oldSeed.Labels = map[string]string{"seed.gardener.cloud/eu-access": "true"}
oldSeed.Spec.AccessRestrictions = []core.AccessRestriction{{Name: "eu-access-only"}}
newSeed.Labels = map[string]string{"seed.gardener.cloud/eu-access": "true"}

strategy.PrepareForUpdate(ctx, newSeed, oldSeed)

Expect(newSeed.Spec.AccessRestrictions).To(BeEmpty())
Expect(newSeed.Labels).To(BeEmpty())
Expect(newSeed.Labels).To(Equal(map[string]string{"seed.gardener.cloud/eu-access": "true"}))
})
})
})
Expand Down
30 changes: 9 additions & 21 deletions pkg/apiserver/registry/core/shoot/strategy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -411,39 +411,27 @@ func syncLegacyAccessRestrictionLabelWithNewFieldOnUpdate(shoot, oldShoot *core.
delete(accessRestrictions[i].Options, key)
}

hasOptionInAccessRestrictions := func(accessRestrictions []core.AccessRestrictionWithOptions, option string) bool {
return slices.ContainsFunc(accessRestrictions, func(accessRestriction core.AccessRestrictionWithOptions) bool {
return accessRestriction.Name == "eu-access-only" && accessRestriction.Options[option] != ""
})
}

if oldShoot.Spec.SeedSelector != nil && oldShoot.Spec.SeedSelector.MatchLabels["seed.gardener.cloud/eu-access"] == "true" &&
(shoot.Spec.SeedSelector == nil || shoot.Spec.SeedSelector.MatchLabels["seed.gardener.cloud/eu-access"] != "true") {
shoot.Spec.AccessRestrictions = removeAccessRestriction(shoot.Spec.AccessRestrictions, "eu-access-only")
}

if hasAccessRestriction(oldShoot.Spec.AccessRestrictions) &&
!hasAccessRestriction(shoot.Spec.AccessRestrictions) {
shoot.Spec.SeedSelector = nil
}

for _, key := range []string{
"support.gardener.cloud/eu-access-for-cluster-addons",
"support.gardener.cloud/eu-access-for-cluster-nodes",
} {
if oldShoot.Annotations[key] != shoot.Annotations[key] {
oldValue, oldOk := oldShoot.Annotations[key]
newValue, newOk := shoot.Annotations[key]
if oldOk && newOk && oldValue != newValue {
updateOptionInAccessRestrictions(shoot.Spec.AccessRestrictions, key, shoot.Annotations[key])
}

if hasAccessRestriction(oldShoot.Spec.AccessRestrictions) &&
hasAccessRestriction(shoot.Spec.AccessRestrictions) &&
oldShoot.Spec.AccessRestrictions[slices.IndexFunc(oldShoot.Spec.AccessRestrictions, filterEUAccessOnlyRestriction)].Options[key] != shoot.Spec.AccessRestrictions[slices.IndexFunc(shoot.Spec.AccessRestrictions, filterEUAccessOnlyRestriction)].Options[key] {
metav1.SetMetaDataAnnotation(&shoot.ObjectMeta, key, shoot.Spec.AccessRestrictions[slices.IndexFunc(shoot.Spec.AccessRestrictions, filterEUAccessOnlyRestriction)].Options[key])
}

if hasOptionInAccessRestrictions(oldShoot.Spec.AccessRestrictions, key) &&
!hasOptionInAccessRestrictions(shoot.Spec.AccessRestrictions, key) {
delete(shoot.Annotations, key)
if hasAccessRestriction(oldShoot.Spec.AccessRestrictions) && hasAccessRestriction(shoot.Spec.AccessRestrictions) {
oldValue, oldOk := oldShoot.Spec.AccessRestrictions[slices.IndexFunc(oldShoot.Spec.AccessRestrictions, filterEUAccessOnlyRestriction)].Options[key]
newValue, newOk := shoot.Spec.AccessRestrictions[slices.IndexFunc(shoot.Spec.AccessRestrictions, filterEUAccessOnlyRestriction)].Options[key]
if oldOk && newOk && oldValue != newValue {
metav1.SetMetaDataAnnotation(&shoot.ObjectMeta, key, shoot.Spec.AccessRestrictions[slices.IndexFunc(shoot.Spec.AccessRestrictions, filterEUAccessOnlyRestriction)].Options[key])
}
}

if oldShoot.Annotations[key] != "" &&
Expand Down
11 changes: 7 additions & 4 deletions pkg/apiserver/registry/core/shoot/strategy_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -671,18 +671,18 @@ var _ = Describe("Strategy", func() {
Expect(newShoot.Spec.SeedSelector).To(BeNil())
})

It("should remove the seed selector when the access restriction is dropped", func() {
It("should not remove the seed selector when the access restriction is dropped", func() {
oldShoot.Spec.SeedSelector = &core.SeedSelector{LabelSelector: metav1.LabelSelector{MatchLabels: map[string]string{"seed.gardener.cloud/eu-access": "true"}}}
oldShoot.Spec.AccessRestrictions = []core.AccessRestrictionWithOptions{{AccessRestriction: core.AccessRestriction{Name: "eu-access-only"}}}
newShoot.Spec.SeedSelector = &core.SeedSelector{LabelSelector: metav1.LabelSelector{MatchLabels: map[string]string{"seed.gardener.cloud/eu-access": "true"}}}

strategy.PrepareForUpdate(context.Background(), newShoot, oldShoot)

Expect(newShoot.Spec.AccessRestrictions).To(BeEmpty())
Expect(newShoot.Spec.SeedSelector).To(BeNil())
Expect(newShoot.Spec.SeedSelector).To(Equal(&core.SeedSelector{LabelSelector: metav1.LabelSelector{MatchLabels: map[string]string{"seed.gardener.cloud/eu-access": "true"}}}))
})

It("should remove the option annotations when they are removed from the access restrictions", func() {
It("should not remove the option annotations when they are removed from the access restrictions", func() {
oldShoot.Spec.AccessRestrictions = []core.AccessRestrictionWithOptions{{
AccessRestriction: core.AccessRestriction{Name: "eu-access-only"},
Options: map[string]string{
Expand All @@ -699,7 +699,10 @@ var _ = Describe("Strategy", func() {

strategy.PrepareForUpdate(context.Background(), newShoot, oldShoot)

Expect(newShoot.Annotations).To(BeEmpty())
Expect(newShoot.Annotations).To(Equal(map[string]string{
"support.gardener.cloud/eu-access-for-cluster-addons": "true",
"support.gardener.cloud/eu-access-for-cluster-nodes": "false",
}))
})

It("should remove the options from the access restrictions when the annotations are removed", func() {
Expand Down

0 comments on commit 4e3b0ac

Please sign in to comment.