Skip to content

Commit

Permalink
gardener-controller-manager: Remove the audit policy configmap refere…
Browse files Browse the repository at this point in the history
…nce protection option (gardener#5525)

Signed-off-by: ialidzhikov <i.alidjikov@gmail.com>
  • Loading branch information
ialidzhikov authored and Kristiyan Gostev committed Jul 5, 2022
1 parent 1aabbc8 commit f970fb9
Show file tree
Hide file tree
Showing 14 changed files with 20 additions and 63 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,6 @@ data:
concurrentSyncs: {{ required ".Values.global.controller.config.controllers.shootHibernation.concurrentSyncs is required" .Values.global.controller.config.controllers.shootHibernation.concurrentSyncs }}
shootReference:
concurrentSyncs: {{ required ".Values.global.controller.config.controllers.shootReference.concurrentSyncs is required" .Values.global.controller.config.controllers.shootReference.concurrentSyncs }}
protectAuditPolicyConfigMaps: {{ required ".Values.global.controller.config.controllers.shootReference.protectAuditPolicyConfigMaps is required" .Values.global.controller.config.controllers.shootReference.protectAuditPolicyConfigMaps }}
shootRetry:
concurrentSyncs: {{ required ".Values.global.controller.config.controllers.shootRetry.concurrentSyncs is required" .Values.global.controller.config.controllers.shootRetry.concurrentSyncs }}
{{- if .Values.global.controller.config.controllers.shootRetry.retryPeriod }}
Expand Down
1 change: 0 additions & 1 deletion charts/gardener/controlplane/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,6 @@ global:
syncPeriod: 24h
shootReference:
concurrentSyncs: 5
protectAuditPolicyConfigMaps: true
shootRetry:
concurrentSyncs: 5
retryPeriod: 10m
Expand Down
5 changes: 1 addition & 4 deletions docs/concepts/controller-manager.md
Original file line number Diff line number Diff line change
Expand Up @@ -117,15 +117,12 @@ Therefore, the Shoot Reference Controller scans shoot clusters for referenced ob
The scanned shoot also gets this finalizer to enable a proper garbage collection in case the Gardener-Controller-Manager is offline at the moment of an incoming deletion request.
When an object is not actively referenced anymore because the shoot specification has changed or all related shoots were deleted (are in deletion), the controller will remove the added finalizer again, so that the object can safely be deleted or garbage collected.

The Shoot Reference Controller can inspect the following references:
The Shoot Reference Controller inspects the following references:
- DNS provider secrets (`.spec.dns.provider`)
- Audit policy configmaps (`.spec.kubernetes.kubeAPIServer.auditConfig.auditPolicy.configMapRef`)

> The audit policy configmap protection is configurable and enabled by default. If you want to disable it then you can set the `.controllers.shootReference.protectAuditPolicyConfigMaps` to `false` in the component configuration.
Further checks might be added in the future.


### Shoot Retry Controller

The Shoot Retry Controller is responsible for retrying certain failed Shoots. Currently the controller retries only failed Shoots with error code `ERR_INFRA_RATE_LIMITS_EXCEEDED`.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ controllers:
syncPeriod: 60m
shootReference:
concurrentSyncs: 5
protectAuditPolicyConfigMaps: true
shootRetry:
concurrentSyncs: 5
# retryDuration: 10m
Expand Down
3 changes: 0 additions & 3 deletions pkg/controllermanager/apis/config/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -264,9 +264,6 @@ type ShootReferenceControllerConfiguration struct {
// ConcurrentSyncs is the number of workers used for the controller to work on
// shoots.
ConcurrentSyncs int
// ProtectAuditPolicyConfigMaps controls whether the shoot reference controller shall protect ConfigMaps containing
// audit policies and referenced in Shoots.
ProtectAuditPolicyConfigMaps *bool
}

// ShootRetryControllerConfiguration defines the configuration of the
Expand Down
4 changes: 0 additions & 4 deletions pkg/controllermanager/apis/config/v1alpha1/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,10 +135,6 @@ func SetDefaults_ControllerManagerConfiguration(obj *ControllerManagerConfigurat
ConcurrentSyncs: 5,
}
}
if obj.Controllers.ShootReference.ProtectAuditPolicyConfigMaps == nil {
v := true
obj.Controllers.ShootReference.ProtectAuditPolicyConfigMaps = &v
}

if obj.Controllers.ShootRetry == nil {
obj.Controllers.ShootRetry = &ShootRetryControllerConfiguration{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@ var _ = Describe("Defaults", func() {

Expect(obj.Controllers.ShootReference).NotTo(BeNil())
Expect(obj.Controllers.ShootReference.ConcurrentSyncs).To(Equal(5))
Expect(obj.Controllers.ShootReference.ProtectAuditPolicyConfigMaps).To(PointTo(BeTrue()))

Expect(obj.Controllers.ShootRetry).NotTo(BeNil())
Expect(obj.Controllers.ShootRetry.ConcurrentSyncs).To(Equal(5))
Expand Down
4 changes: 0 additions & 4 deletions pkg/controllermanager/apis/config/v1alpha1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -298,10 +298,6 @@ type ShootReferenceControllerConfiguration struct {
// ConcurrentSyncs is the number of workers used for the controller to work on
// shoots.
ConcurrentSyncs int `json:"concurrentSyncs"`
// ProtectAuditPolicyConfigMaps controls whether the shoot reference controller shall protect ConfigMaps containing
// audit policies and referenced in Shoots.
// +optional
ProtectAuditPolicyConfigMaps *bool `json:"protectAuditPolicyConfigMaps,omitempty"`
}

// ShootRetryControllerConfiguration defines the configuration of the
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 1 addition & 6 deletions pkg/controllermanager/apis/config/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pkg/controllermanager/controller/shoot/shoot.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ func NewShootController(
shootRetryReconciler: NewShootRetryReconciler(logger.Logger, gardenClient.Client(), config.Controllers.ShootRetry),
shootConditionsReconciler: NewShootConditionsReconciler(logger.Logger, gardenClient.Client()),
shootStatusLabelReconciler: NewShootStatusLabelReconciler(logger.Logger, gardenClient.Client()),
shootRefReconciler: NewShootReferenceReconciler(logger.Logger, gardenClient, config.Controllers.ShootReference),
shootRefReconciler: NewShootReferenceReconciler(logger.Logger, gardenClient),

shootMaintenanceQueue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "shoot-maintenance"),
shootQuotaQueue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "shoot-quota"),
Expand Down
38 changes: 15 additions & 23 deletions pkg/controllermanager/controller/shoot/shoot_reference_control.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
gardencorev1beta1 "github.com/gardener/gardener/pkg/apis/core/v1beta1"
v1beta1constants "github.com/gardener/gardener/pkg/apis/core/v1beta1/constants"
"github.com/gardener/gardener/pkg/client/kubernetes"
"github.com/gardener/gardener/pkg/controllermanager/apis/config"
"github.com/gardener/gardener/pkg/controllerutils"
gardenlogger "github.com/gardener/gardener/pkg/logger"
"github.com/gardener/gardener/pkg/utils"
Expand Down Expand Up @@ -56,7 +55,7 @@ func (c *Controller) shootReferenceUpdate(oldObj, newObj interface{}) {
newShoot = newObj.(*gardencorev1beta1.Shoot)
)

if c.refChange(oldShoot, newShoot) || newShoot.DeletionTimestamp != nil && !controllerutil.ContainsFinalizer(newShoot, gardencorev1beta1.GardenerName) {
if refChange(oldShoot, newShoot) || newShoot.DeletionTimestamp != nil && !controllerutil.ContainsFinalizer(newShoot, gardencorev1beta1.GardenerName) {
key, err := cache.MetaNamespaceKeyFunc(newObj)
if err != nil {
gardenlogger.Logger.Errorf("Couldn't get key for object %+v: %v", newObj, err)
Expand All @@ -66,9 +65,8 @@ func (c *Controller) shootReferenceUpdate(oldObj, newObj interface{}) {
}
}

func (c *Controller) refChange(oldShoot, newShoot *gardencorev1beta1.Shoot) bool {
return shootDNSFieldChanged(oldShoot, newShoot) ||
(utils.IsTrue(c.config.Controllers.ShootReference.ProtectAuditPolicyConfigMaps) && shootKubeAPIServerAuditConfigFieldChanged(oldShoot, newShoot))
func refChange(oldShoot, newShoot *gardencorev1beta1.Shoot) bool {
return shootDNSFieldChanged(oldShoot, newShoot) || shootKubeAPIServerAuditConfigFieldChanged(oldShoot, newShoot)
}

func shootDNSFieldChanged(oldShoot, newShoot *gardencorev1beta1.Shoot) bool {
Expand All @@ -83,18 +81,16 @@ func shootKubeAPIServerAuditConfigFieldChanged(oldShoot, newShoot *gardencorev1b
const FinalizerName = "gardener.cloud/reference-protection"

// NewShootReferenceReconciler creates a new instance of a reconciler which checks object references from shoot objects.
func NewShootReferenceReconciler(l logrus.FieldLogger, gardenClient kubernetes.Interface, config *config.ShootReferenceControllerConfiguration) reconcile.Reconciler {
func NewShootReferenceReconciler(l logrus.FieldLogger, gardenClient kubernetes.Interface) reconcile.Reconciler {
return &shootReferenceReconciler{
gardenClient: gardenClient,
logger: l,
config: config,
}
}

type shootReferenceReconciler struct {
logger logrus.FieldLogger
gardenClient kubernetes.Interface
config *config.ShootReferenceControllerConfiguration
}

// Reconcile checks the shoot in the given request for references to further objects in order to protect them from
Expand Down Expand Up @@ -190,19 +186,17 @@ func (r *shootReferenceReconciler) handleReferencedSecrets(ctx context.Context,
}

func (r *shootReferenceReconciler) handleReferencedConfigMap(ctx context.Context, c client.Client, shoot *gardencorev1beta1.Shoot) (bool, error) {
if utils.IsTrue(r.config.ProtectAuditPolicyConfigMaps) {
if configMapRef := getAuditPolicyConfigMapRef(shoot.Spec.Kubernetes.KubeAPIServer); configMapRef != nil {
configMap := &corev1.ConfigMap{}
if err := c.Get(ctx, kutil.Key(shoot.Namespace, configMapRef.Name), configMap); err != nil {
return false, err
}

if controllerutil.ContainsFinalizer(configMap, FinalizerName) {
return true, nil
}
if configMapRef := getAuditPolicyConfigMapRef(shoot.Spec.Kubernetes.KubeAPIServer); configMapRef != nil {
configMap := &corev1.ConfigMap{}
if err := c.Get(ctx, kutil.Key(shoot.Namespace, configMapRef.Name), configMap); err != nil {
return false, err
}

return true, controllerutils.StrategicMergePatchAddFinalizers(ctx, c, configMap, FinalizerName)
if controllerutil.ContainsFinalizer(configMap, FinalizerName) {
return true, nil
}

return true, controllerutils.StrategicMergePatchAddFinalizers(ctx, c, configMap, FinalizerName)
}

return false, nil
Expand Down Expand Up @@ -310,10 +304,8 @@ func (r *shootReferenceReconciler) getUnreferencedConfigMaps(ctx context.Context
continue
}

if utils.IsTrue(r.config.ProtectAuditPolicyConfigMaps) {
if configMapRef := getAuditPolicyConfigMapRef(s.Spec.Kubernetes.KubeAPIServer); configMapRef != nil {
referencedConfigMaps.Insert(configMapRef.Name)
}
if configMapRef := getAuditPolicyConfigMapRef(s.Spec.Kubernetes.KubeAPIServer); configMapRef != nil {
referencedConfigMaps.Insert(configMapRef.Name)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (

gardencorev1beta1 "github.com/gardener/gardener/pkg/apis/core/v1beta1"
fakeclientset "github.com/gardener/gardener/pkg/client/kubernetes/fake"
"github.com/gardener/gardener/pkg/controllermanager/apis/config"
. "github.com/gardener/gardener/pkg/controllermanager/controller/shoot"
"github.com/gardener/gardener/pkg/logger"
mockcache "github.com/gardener/gardener/pkg/mock/controller-runtime/cache"
Expand Down Expand Up @@ -55,7 +54,6 @@ var _ = Describe("Shoot References", func() {
reconciler reconcile.Reconciler
shoot gardencorev1beta1.Shoot
gardenClient *fakeclientset.ClientSet
cfg = &config.ShootReferenceControllerConfiguration{}
)

BeforeEach(func() {
Expand All @@ -78,7 +76,7 @@ var _ = Describe("Shoot References", func() {
})

JustBeforeEach(func() {
reconciler = NewShootReferenceReconciler(logger.NewNopLogger(), gardenClient, cfg)
reconciler = NewShootReferenceReconciler(logger.NewNopLogger(), gardenClient)
})

Context("Common controller tests", func() {
Expand Down Expand Up @@ -458,9 +456,6 @@ var _ = Describe("Shoot References", func() {
var configMaps []corev1.ConfigMap

BeforeEach(func() {
cfg.ProtectAuditPolicyConfigMaps = pointer.Bool(true)
reconciler = NewShootReferenceReconciler(logger.NewNopLogger(), gardenClient, cfg)

configMaps = []corev1.ConfigMap{
{ObjectMeta: metav1.ObjectMeta{
Name: "configmap-1",
Expand Down

0 comments on commit f970fb9

Please sign in to comment.