From dfe38788d0b7623a21722824b5d62a8820775707 Mon Sep 17 00:00:00 2001 From: Avinash Patnala Date: Sat, 14 Sep 2024 03:30:43 +0000 Subject: [PATCH 01/28] add config pod status crd Signed-off-by: Avinash Patnala --- apis/config/v1alpha1/config_types.go | 4 + apis/config/v1alpha1/zz_generated.deepcopy.go | 10 +- apis/status/v1beta1/configpodstatus_types.go | 89 +++++++++++++++ apis/status/v1beta1/labels.go | 1 + apis/status/v1beta1/zz_generated.deepcopy.go | 104 ++++++++++++++++++ .../bases/config.gatekeeper.sh_configs.yaml | 36 ++++++ ...tatus.gatekeeper.sh_configpodstatuses.yaml | 71 ++++++++++++ config/crd/kustomization.yaml | 1 + .../crds/config-customresourcedefinition.yaml | 35 ++++++ manifest_staging/deploy/gatekeeper.yaml | 35 ++++++ 10 files changed, 385 insertions(+), 1 deletion(-) create mode 100644 apis/status/v1beta1/configpodstatus_types.go create mode 100644 config/crd/bases/status.gatekeeper.sh_configpodstatuses.yaml diff --git a/apis/config/v1alpha1/config_types.go b/apis/config/v1alpha1/config_types.go index 372e496c511..672358e1457 100644 --- a/apis/config/v1alpha1/config_types.go +++ b/apis/config/v1alpha1/config_types.go @@ -16,6 +16,7 @@ limitations under the License. package v1alpha1 import ( + status "github.com/open-policy-agent/gatekeeper/v3/apis/status/v1beta1" "github.com/open-policy-agent/gatekeeper/v3/pkg/wildcard" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" @@ -82,6 +83,7 @@ type ReadinessSpec struct { // ConfigStatus defines the observed state of Config. type ConfigStatus struct { // Important: Run "make" to regenerate code after modifying this file + ByPod []status.ConfigPodStatusStatus `json:"byPod,omitempty"` } type GVK struct { @@ -92,6 +94,8 @@ type GVK struct { // +kubebuilder:resource:scope=Namespaced // +kubebuilder:object:root=true +// +kubebuilder:subresource:status +// +kubebuilder:storageversion // Config is the Schema for the configs API. type Config struct { diff --git a/apis/config/v1alpha1/zz_generated.deepcopy.go b/apis/config/v1alpha1/zz_generated.deepcopy.go index 75babe05f76..c60bc2cda4f 100644 --- a/apis/config/v1alpha1/zz_generated.deepcopy.go +++ b/apis/config/v1alpha1/zz_generated.deepcopy.go @@ -20,6 +20,7 @@ limitations under the License. package v1alpha1 import ( + "github.com/open-policy-agent/gatekeeper/v3/apis/status/v1beta1" "github.com/open-policy-agent/gatekeeper/v3/pkg/wildcard" runtime "k8s.io/apimachinery/pkg/runtime" ) @@ -30,7 +31,7 @@ func (in *Config) DeepCopyInto(out *Config) { out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) in.Spec.DeepCopyInto(&out.Spec) - out.Status = in.Status + in.Status.DeepCopyInto(&out.Status) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Config. @@ -111,6 +112,13 @@ func (in *ConfigSpec) DeepCopy() *ConfigSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ConfigStatus) DeepCopyInto(out *ConfigStatus) { *out = *in + if in.ByPod != nil { + in, out := &in.ByPod, &out.ByPod + *out = make([]v1beta1.ConfigPodStatusStatus, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ConfigStatus. diff --git a/apis/status/v1beta1/configpodstatus_types.go b/apis/status/v1beta1/configpodstatus_types.go new file mode 100644 index 00000000000..713688508ed --- /dev/null +++ b/apis/status/v1beta1/configpodstatus_types.go @@ -0,0 +1,89 @@ +package v1beta1 + +import ( + "github.com/open-policy-agent/gatekeeper/v3/pkg/operations" + "github.com/open-policy-agent/gatekeeper/v3/pkg/util" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + + // "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" +) + +// ConfigPodStatusStatus defines the observed state of ConfigPodStatus. + +// +kubebuilder:object:generate=true + +type ConfigPodStatusStatus struct { + // Important: Run "make" to regenerate code after modifying this file + ID string `json:"id,omitempty"` + ConfigUID types.UID `json:"configUID,omitempty"` + Operations []string `json:"operations,omitempty"` + ObservedGeneration int64 `json:"observedGeneration,omitempty"` + Errors []*ConfigError `json:"errors,omitempty"` +} + +// +kubebuilder:object:generate=true + +type ConfigError struct { + Type string `json:"type,omitempty"` + Message string `json:"message"` +} + +// ConfigPodStatus is the Schema for the configpodstatuses API. + +// +kubebuilder:object:root=true +// +kubebuilder:resource:scope=Namespaced + +type ConfigPodStatus struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` + + Status ConfigPodStatusStatus `json:"status,omitempty"` +} + +// ConfigPodStatusList contains a list of ConfigPodStatus. + +// +kubebuilder:object:root=true +type ConfigPodStatusList struct { + metav1.TypeMeta `json:",inline"` + metav1.ListMeta `json:"metadata,omitempty"` + Items []ConfigPodStatus `json:"items"` +} + +func init() { + SchemeBuilder.Register(&ConfigPodStatus{}, &ConfigPodStatusList{}) +} + +// NewConfigStatusForPod returns an config status object +// that has been initialized with the bare minimum of fields to make it functional +// with the config status controller. +func NewConfigStatusForPod(pod *corev1.Pod, configName string, scheme *runtime.Scheme) (*ConfigPodStatus, error) { + obj := &ConfigPodStatus{} + name, err := KeyForConfig(pod.Name, configName) + if err != nil { + return nil, err + } + obj.SetName(name) + obj.SetNamespace(util.GetNamespace()) + obj.Status.ID = pod.Name + obj.Status.Operations = operations.AssignedStringList() + obj.SetLabels(map[string]string{ + ConfigNameLabel: configName, + PodLabel: pod.Name, + }) + + if err := controllerutil.SetOwnerReference(pod, obj, scheme); err != nil { + return nil, err + } + + return obj, nil +} + +// KeyForConfig returns a unique status object name given the Pod ID and +// a config object. +func KeyForConfig(id string, configName string) (string, error) { + return DashPacker(id, configName) +} diff --git a/apis/status/v1beta1/labels.go b/apis/status/v1beta1/labels.go index 0f0caca91ce..61f3a7f384f 100644 --- a/apis/status/v1beta1/labels.go +++ b/apis/status/v1beta1/labels.go @@ -2,6 +2,7 @@ package v1beta1 // Label keys used for internal gatekeeper operations. const ( + ConfigNameLabel = "internal.gatekeeper.sh/config-name" ExpansionTemplateNameLabel = "internal.gatekeeper.sh/expansiontemplate-name" ConstraintNameLabel = "internal.gatekeeper.sh/constraint-name" ConstraintKindLabel = "internal.gatekeeper.sh/constraint-kind" diff --git a/apis/status/v1beta1/zz_generated.deepcopy.go b/apis/status/v1beta1/zz_generated.deepcopy.go index c361b6cdd9a..fa401e6ced1 100644 --- a/apis/status/v1beta1/zz_generated.deepcopy.go +++ b/apis/status/v1beta1/zz_generated.deepcopy.go @@ -24,6 +24,110 @@ import ( "k8s.io/apimachinery/pkg/runtime" ) +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ConfigError) DeepCopyInto(out *ConfigError) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ConfigError. +func (in *ConfigError) DeepCopy() *ConfigError { + if in == nil { + return nil + } + out := new(ConfigError) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ConfigPodStatus) DeepCopyInto(out *ConfigPodStatus) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) + in.Status.DeepCopyInto(&out.Status) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ConfigPodStatus. +func (in *ConfigPodStatus) DeepCopy() *ConfigPodStatus { + if in == nil { + return nil + } + out := new(ConfigPodStatus) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *ConfigPodStatus) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ConfigPodStatusList) DeepCopyInto(out *ConfigPodStatusList) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ListMeta.DeepCopyInto(&out.ListMeta) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]ConfigPodStatus, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ConfigPodStatusList. +func (in *ConfigPodStatusList) DeepCopy() *ConfigPodStatusList { + if in == nil { + return nil + } + out := new(ConfigPodStatusList) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *ConfigPodStatusList) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ConfigPodStatusStatus) DeepCopyInto(out *ConfigPodStatusStatus) { + *out = *in + if in.Operations != nil { + in, out := &in.Operations, &out.Operations + *out = make([]string, len(*in)) + copy(*out, *in) + } + if in.Errors != nil { + in, out := &in.Errors, &out.Errors + *out = make([]*ConfigError, len(*in)) + for i := range *in { + if (*in)[i] != nil { + in, out := &(*in)[i], &(*out)[i] + *out = new(ConfigError) + **out = **in + } + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ConfigPodStatusStatus. +func (in *ConfigPodStatusStatus) DeepCopy() *ConfigPodStatusStatus { + if in == nil { + return nil + } + out := new(ConfigPodStatusStatus) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ConstraintPodStatus) DeepCopyInto(out *ConstraintPodStatus) { *out = *in diff --git a/config/crd/bases/config.gatekeeper.sh_configs.yaml b/config/crd/bases/config.gatekeeper.sh_configs.yaml index ddd2f55394f..2bed80a7eb0 100644 --- a/config/crd/bases/config.gatekeeper.sh_configs.yaml +++ b/config/crd/bases/config.gatekeeper.sh_configs.yaml @@ -112,7 +112,43 @@ spec: type: object status: description: ConfigStatus defines the observed state of Config. + properties: + byPod: + items: + properties: + configUID: + description: |- + UID is a type that holds unique ID values, including UUIDs. Because we + don't ONLY use UUIDs, this is an alias to string. Being a type captures + intent and helps make sure that UIDs and names do not get conflated. + type: string + errors: + items: + properties: + message: + type: string + type: + type: string + required: + - message + type: object + type: array + id: + description: 'Important: Run "make" to regenerate code after + modifying this file' + type: string + observedGeneration: + format: int64 + type: integer + operations: + items: + type: string + type: array + type: object + type: array type: object type: object served: true storage: true + subresources: + status: {} diff --git a/config/crd/bases/status.gatekeeper.sh_configpodstatuses.yaml b/config/crd/bases/status.gatekeeper.sh_configpodstatuses.yaml new file mode 100644 index 00000000000..57604c6083e --- /dev/null +++ b/config/crd/bases/status.gatekeeper.sh_configpodstatuses.yaml @@ -0,0 +1,71 @@ +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.14.0 + name: configpodstatuses.status.gatekeeper.sh +spec: + group: status.gatekeeper.sh + names: + kind: ConfigPodStatus + listKind: ConfigPodStatusList + plural: configpodstatuses + singular: configpodstatus + scope: Namespaced + versions: + - name: v1beta1 + schema: + openAPIV3Schema: + properties: + apiVersion: + description: |- + APIVersion defines the versioned schema of this representation of an object. + Servers should convert recognized schemas to the latest internal value, and + may reject unrecognized values. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources + type: string + kind: + description: |- + Kind is a string value representing the REST resource this object represents. + Servers may infer this from the endpoint the client submits requests to. + Cannot be updated. + In CamelCase. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds + type: string + metadata: + type: object + status: + properties: + configUID: + description: |- + UID is a type that holds unique ID values, including UUIDs. Because we + don't ONLY use UUIDs, this is an alias to string. Being a type captures + intent and helps make sure that UIDs and names do not get conflated. + type: string + errors: + items: + properties: + message: + type: string + type: + type: string + required: + - message + type: object + type: array + id: + description: 'Important: Run "make" to regenerate code after modifying + this file' + type: string + observedGeneration: + format: int64 + type: integer + operations: + items: + type: string + type: array + type: object + type: object + served: true + storage: true diff --git a/config/crd/kustomization.yaml b/config/crd/kustomization.yaml index 4eb253c7672..568b63293b9 100644 --- a/config/crd/kustomization.yaml +++ b/config/crd/kustomization.yaml @@ -8,6 +8,7 @@ resources: - bases/status.gatekeeper.sh_constrainttemplatepodstatuses.yaml - bases/status.gatekeeper.sh_mutatorpodstatuses.yaml - bases/status.gatekeeper.sh_expansiontemplatepodstatuses.yaml +- bases/status.gatekeeper.sh_configpodstatuses.yaml - bases/mutations.gatekeeper.sh_assign.yaml - bases/mutations.gatekeeper.sh_assignimage.yaml - bases/mutations.gatekeeper.sh_assignmetadata.yaml diff --git a/manifest_staging/charts/gatekeeper/crds/config-customresourcedefinition.yaml b/manifest_staging/charts/gatekeeper/crds/config-customresourcedefinition.yaml index 11a5d922789..5182e6625a2 100644 --- a/manifest_staging/charts/gatekeeper/crds/config-customresourcedefinition.yaml +++ b/manifest_staging/charts/gatekeeper/crds/config-customresourcedefinition.yaml @@ -112,7 +112,42 @@ spec: type: object status: description: ConfigStatus defines the observed state of Config. + properties: + byPod: + items: + properties: + configUID: + description: |- + UID is a type that holds unique ID values, including UUIDs. Because we + don't ONLY use UUIDs, this is an alias to string. Being a type captures + intent and helps make sure that UIDs and names do not get conflated. + type: string + errors: + items: + properties: + message: + type: string + type: + type: string + required: + - message + type: object + type: array + id: + description: 'Important: Run "make" to regenerate code after modifying this file' + type: string + observedGeneration: + format: int64 + type: integer + operations: + items: + type: string + type: array + type: object + type: array type: object type: object served: true storage: true + subresources: + status: {} diff --git a/manifest_staging/deploy/gatekeeper.yaml b/manifest_staging/deploy/gatekeeper.yaml index fe375e4eb43..a868bc5df07 100644 --- a/manifest_staging/deploy/gatekeeper.yaml +++ b/manifest_staging/deploy/gatekeeper.yaml @@ -2498,10 +2498,45 @@ spec: type: object status: description: ConfigStatus defines the observed state of Config. + properties: + byPod: + items: + properties: + configUID: + description: |- + UID is a type that holds unique ID values, including UUIDs. Because we + don't ONLY use UUIDs, this is an alias to string. Being a type captures + intent and helps make sure that UIDs and names do not get conflated. + type: string + errors: + items: + properties: + message: + type: string + type: + type: string + required: + - message + type: object + type: array + id: + description: 'Important: Run "make" to regenerate code after modifying this file' + type: string + observedGeneration: + format: int64 + type: integer + operations: + items: + type: string + type: array + type: object + type: array type: object type: object served: true storage: true + subresources: + status: {} --- apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition From 14d80aa64dddaac8dd87addc577c51311d86d944 Mon Sep 17 00:00:00 2001 From: Avinash Patnala Date: Mon, 16 Sep 2024 18:44:45 +0000 Subject: [PATCH 02/28] generate manifests Signed-off-by: Avinash Patnala --- apis/status/v1beta1/configpodstatus_types.go | 2 - ...figpodstatus-customresourcedefinition.yaml | 77 +++++++++++++++++++ manifest_staging/deploy/gatekeeper.yaml | 73 ++++++++++++++++++ 3 files changed, 150 insertions(+), 2 deletions(-) create mode 100644 manifest_staging/charts/gatekeeper/crds/configpodstatus-customresourcedefinition.yaml diff --git a/apis/status/v1beta1/configpodstatus_types.go b/apis/status/v1beta1/configpodstatus_types.go index 713688508ed..6160355ab66 100644 --- a/apis/status/v1beta1/configpodstatus_types.go +++ b/apis/status/v1beta1/configpodstatus_types.go @@ -6,8 +6,6 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" - - // "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" ) diff --git a/manifest_staging/charts/gatekeeper/crds/configpodstatus-customresourcedefinition.yaml b/manifest_staging/charts/gatekeeper/crds/configpodstatus-customresourcedefinition.yaml new file mode 100644 index 00000000000..750eb39e62b --- /dev/null +++ b/manifest_staging/charts/gatekeeper/crds/configpodstatus-customresourcedefinition.yaml @@ -0,0 +1,77 @@ +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.14.0 + labels: + app: '{{ template "gatekeeper.name" . }}' + chart: '{{ template "gatekeeper.name" . }}' + gatekeeper.sh/system: "yes" + heritage: '{{ .Release.Service }}' + release: '{{ .Release.Name }}' + name: configpodstatuses.status.gatekeeper.sh +spec: + group: status.gatekeeper.sh + names: + kind: ConfigPodStatus + listKind: ConfigPodStatusList + plural: configpodstatuses + singular: configpodstatus + preserveUnknownFields: false + scope: Namespaced + versions: + - name: v1beta1 + schema: + openAPIV3Schema: + properties: + apiVersion: + description: |- + APIVersion defines the versioned schema of this representation of an object. + Servers should convert recognized schemas to the latest internal value, and + may reject unrecognized values. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources + type: string + kind: + description: |- + Kind is a string value representing the REST resource this object represents. + Servers may infer this from the endpoint the client submits requests to. + Cannot be updated. + In CamelCase. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds + type: string + metadata: + type: object + status: + properties: + configUID: + description: |- + UID is a type that holds unique ID values, including UUIDs. Because we + don't ONLY use UUIDs, this is an alias to string. Being a type captures + intent and helps make sure that UIDs and names do not get conflated. + type: string + errors: + items: + properties: + message: + type: string + type: + type: string + required: + - message + type: object + type: array + id: + description: 'Important: Run "make" to regenerate code after modifying this file' + type: string + observedGeneration: + format: int64 + type: integer + operations: + items: + type: string + type: array + type: object + type: object + served: true + storage: true diff --git a/manifest_staging/deploy/gatekeeper.yaml b/manifest_staging/deploy/gatekeeper.yaml index a868bc5df07..575f701c928 100644 --- a/manifest_staging/deploy/gatekeeper.yaml +++ b/manifest_staging/deploy/gatekeeper.yaml @@ -2387,6 +2387,79 @@ spec: --- apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.14.0 + labels: + gatekeeper.sh/system: "yes" + name: configpodstatuses.status.gatekeeper.sh +spec: + group: status.gatekeeper.sh + names: + kind: ConfigPodStatus + listKind: ConfigPodStatusList + plural: configpodstatuses + singular: configpodstatus + preserveUnknownFields: false + scope: Namespaced + versions: + - name: v1beta1 + schema: + openAPIV3Schema: + properties: + apiVersion: + description: |- + APIVersion defines the versioned schema of this representation of an object. + Servers should convert recognized schemas to the latest internal value, and + may reject unrecognized values. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources + type: string + kind: + description: |- + Kind is a string value representing the REST resource this object represents. + Servers may infer this from the endpoint the client submits requests to. + Cannot be updated. + In CamelCase. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds + type: string + metadata: + type: object + status: + properties: + configUID: + description: |- + UID is a type that holds unique ID values, including UUIDs. Because we + don't ONLY use UUIDs, this is an alias to string. Being a type captures + intent and helps make sure that UIDs and names do not get conflated. + type: string + errors: + items: + properties: + message: + type: string + type: + type: string + required: + - message + type: object + type: array + id: + description: 'Important: Run "make" to regenerate code after modifying this file' + type: string + observedGeneration: + format: int64 + type: integer + operations: + items: + type: string + type: array + type: object + type: object + served: true + storage: true +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition metadata: annotations: controller-gen.kubebuilder.io/version: v0.14.0 From 369004c8931a9af262933ed10de22b296f9704b5 Mon Sep 17 00:00:00 2001 From: Avinash Patnala Date: Mon, 16 Sep 2024 23:40:39 +0000 Subject: [PATCH 03/28] implement config status controller wip Signed-off-by: Avinash Patnala --- pkg/controller/add_configstatus.go | 21 ++ pkg/controller/config/config_controller.go | 14 +- .../config/config_controller_test.go | 21 +- .../configstatus/configstatus_controller.go | 195 ++++++++++++++++++ 4 files changed, 246 insertions(+), 5 deletions(-) create mode 100644 pkg/controller/add_configstatus.go create mode 100644 pkg/controller/configstatus/configstatus_controller.go diff --git a/pkg/controller/add_configstatus.go b/pkg/controller/add_configstatus.go new file mode 100644 index 00000000000..4dfded79e01 --- /dev/null +++ b/pkg/controller/add_configstatus.go @@ -0,0 +1,21 @@ +/* +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + http://www.apache.org/licenses/LICENSE-2.0 +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "github.com/open-policy-agent/gatekeeper/v3/pkg/controller/configstatus" +) + +func init() { + Injectors = append(Injectors, &configstatus.Adder{}) +} diff --git a/pkg/controller/config/config_controller.go b/pkg/controller/config/config_controller.go index 4cfa78e27b6..49bb06963a6 100644 --- a/pkg/controller/config/config_controller.go +++ b/pkg/controller/config/config_controller.go @@ -26,6 +26,7 @@ import ( "github.com/open-policy-agent/gatekeeper/v3/pkg/keys" "github.com/open-policy-agent/gatekeeper/v3/pkg/readiness" "github.com/open-policy-agent/gatekeeper/v3/pkg/watch" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" @@ -51,12 +52,14 @@ type Adder struct { ControllerSwitch *watch.ControllerSwitch Tracker *readiness.Tracker CacheManager *cm.CacheManager + // GetPod returns an instance of the currently running Gatekeeper pod + GetPod func(context.Context) (*corev1.Pod, error) } // Add creates a new ConfigController and adds it to the Manager with default RBAC. The Manager will set fields on the Controller // and Start it when the Manager is Started. func (a *Adder) Add(mgr manager.Manager) error { - r, err := newReconciler(mgr, a.CacheManager, a.ControllerSwitch, a.Tracker) + r, err := newReconciler(mgr, a.CacheManager, a.ControllerSwitch, a.Tracker, a.GetPod) if err != nil { return err } @@ -76,8 +79,12 @@ func (a *Adder) InjectCacheManager(cm *cm.CacheManager) { a.CacheManager = cm } +func (a *Adder) InjectGetPod(getPod func(ctx context.Context) (*corev1.Pod, error)) { + a.GetPod = getPod +} + // newReconciler returns a new reconcile.Reconciler. -func newReconciler(mgr manager.Manager, cm *cm.CacheManager, cs *watch.ControllerSwitch, tracker *readiness.Tracker) (*ReconcileConfig, error) { +func newReconciler(mgr manager.Manager, cm *cm.CacheManager, cs *watch.ControllerSwitch, tracker *readiness.Tracker, getPod func(context.Context) (*corev1.Pod, error)) (*ReconcileConfig, error) { if cm == nil { return nil, fmt.Errorf("cacheManager must be non-nil") } @@ -90,6 +97,7 @@ func newReconciler(mgr manager.Manager, cm *cm.CacheManager, cs *watch.Controlle cs: cs, cacheManager: cm, tracker: tracker, + getPod: getPod, }, nil } @@ -123,6 +131,8 @@ type ReconcileConfig struct { cs *watch.ControllerSwitch tracker *readiness.Tracker + + getPod func(context.Context) (*corev1.Pod, error) } // +kubebuilder:rbac:groups=*,resources=*,verbs=get;list;watch diff --git a/pkg/controller/config/config_controller_test.go b/pkg/controller/config/config_controller_test.go index 6de6361ea00..a44b6462436 100644 --- a/pkg/controller/config/config_controller_test.go +++ b/pkg/controller/config/config_controller_test.go @@ -154,7 +154,12 @@ func TestReconcile(t *testing.T) { assert.NoError(t, cacheManager.Start(ctx)) }() - rec, err := newReconciler(mgr, cacheManager, cs, tracker) + pod := fakes.Pod( + fakes.WithNamespace("gatekeeper-system"), + fakes.WithName("no-pod"), + ) + + rec, err := newReconciler(mgr, cacheManager, cs, tracker, func(context.Context) (*corev1.Pod, error) { return pod, nil }) require.NoError(t, err) // Wrap the Controller Reconcile function so it writes each request to a map when it is finished reconciling. @@ -444,7 +449,12 @@ func setupController(ctx context.Context, mgr manager.Manager, wm *watch.Manager _ = cacheManager.Start(ctx) }() - rec, err := newReconciler(mgr, cacheManager, cs, tracker) + pod := fakes.Pod( + fakes.WithNamespace("gatekeeper-system"), + fakes.WithName("no-pod"), + ) + + rec, err := newReconciler(mgr, cacheManager, cs, tracker, func(context.Context) (*corev1.Pod, error) { return pod, nil }) if err != nil { return nil, fmt.Errorf("creating reconciler: %w", err) } @@ -618,7 +628,12 @@ func TestConfig_Retries(t *testing.T) { assert.NoError(t, cacheManager.Start(ctx)) }() - rec, _ := newReconciler(mgr, cacheManager, cs, tracker) + pod := fakes.Pod( + fakes.WithNamespace("gatekeeper-system"), + fakes.WithName("no-pod"), + ) + + rec, _ := newReconciler(mgr, cacheManager, cs, tracker, func(context.Context) (*corev1.Pod, error) { return pod, nil }) err = add(mgr, rec) if err != nil { t.Fatal(err) diff --git a/pkg/controller/configstatus/configstatus_controller.go b/pkg/controller/configstatus/configstatus_controller.go new file mode 100644 index 00000000000..7f91cb80cbb --- /dev/null +++ b/pkg/controller/configstatus/configstatus_controller.go @@ -0,0 +1,195 @@ +/* + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package configstatus + +import ( + "context" + "fmt" + "sort" + + "github.com/go-logr/logr" + configv1alpha1 "github.com/open-policy-agent/gatekeeper/v3/apis/config/v1alpha1" + "github.com/open-policy-agent/gatekeeper/v3/apis/status/v1beta1" + "github.com/open-policy-agent/gatekeeper/v3/pkg/logging" + "github.com/open-policy-agent/gatekeeper/v3/pkg/readiness" + "github.com/open-policy-agent/gatekeeper/v3/pkg/util" + "github.com/open-policy-agent/gatekeeper/v3/pkg/watch" + "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller" + "sigs.k8s.io/controller-runtime/pkg/handler" + logf "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + "sigs.k8s.io/controller-runtime/pkg/source" +) + +var log = logf.Log.WithName("controller").WithValues(logging.Process, "config_status_controller") + +type Adder struct { + WatchManager *watch.Manager +} + +func (a *Adder) InjectControllerSwitch(_ *watch.ControllerSwitch) {} + +func (a *Adder) InjectTracker(_ *readiness.Tracker) {} + +// Add creates a new config Status Controller and adds it to the Manager. The Manager will set fields on the Controller +// and Start it when the Manager is Started. +func (a *Adder) Add(mgr manager.Manager) error { + r := newReconciler(mgr) + return add(mgr, r) +} + +// newReconciler returns a new reconcile.Reconciler. +func newReconciler(mgr manager.Manager) reconcile.Reconciler { + return &ReconcileConfigStatus{ + // Separate reader and writer because manager's default client bypasses the cache for unstructured resources. + writer: mgr.GetClient(), + statusClient: mgr.GetClient(), + reader: mgr.GetCache(), + + scheme: mgr.GetScheme(), + log: log, + } +} + +// PodStatusToConfigMapper correlates a ConfigPodStatus with its corresponding Config. +// `selfOnly` tells the mapper to only map statuses corresponding to the current pod. +func PodStatusToConfigMapper(selfOnly bool) handler.TypedMapFunc[*v1beta1.ConfigPodStatus] { + return func(_ context.Context, obj *v1beta1.ConfigPodStatus) []reconcile.Request { + labels := obj.GetLabels() + name, ok := labels[v1beta1.ConfigNameLabel] + if !ok { + log.Error(fmt.Errorf("config status resource with no mapping label: %s", obj.GetName()), "missing label while attempting to map a config status resource") + return nil + } + if selfOnly { + pod, ok := labels[v1beta1.PodLabel] + if !ok { + log.Error(fmt.Errorf("config status resource with no pod label: %s", obj.GetName()), "missing label while attempting to map a config status resource") + } + // Do not attempt to reconcile the resource when other pods have changed their status + if pod != util.GetPodName() { + return nil + } + } + return []reconcile.Request{{NamespacedName: types.NamespacedName{Name: name}}} + } +} + +// add adds a new Controller to mgr with r as the reconcile.Reconciler. +func add(mgr manager.Manager, r reconcile.Reconciler) error { + // Create a new controller + c, err := controller.New("config-status-controller", mgr, controller.Options{Reconciler: r}) + if err != nil { + return err + } + + // Watch for changes to ExpansionTemplateStatus + err = c.Watch( + source.Kind(mgr.GetCache(), &v1beta1.ConfigPodStatus{}, + handler.TypedEnqueueRequestsFromMapFunc(PodStatusToConfigMapper(false)), + )) + if err != nil { + return err + } + + // Watch for changes to ExpansionTemplate + err = c.Watch(source.Kind(mgr.GetCache(), &configv1alpha1.Config{}, &handler.TypedEnqueueRequestForObject[*configv1alpha1.Config]{})) + if err != nil { + return err + } + return nil +} + +var _ reconcile.Reconciler = &ReconcileConfigStatus{} + +// ReconcileConfigStatus reconciles an arbitrary config object described by Kind. +type ReconcileConfigStatus struct { + reader client.Reader + writer client.Writer + statusClient client.StatusClient + + scheme *runtime.Scheme + log logr.Logger +} + +// +kubebuilder:rbac:groups=expansion.gatekeeper.sh,resources=*,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups=status.gatekeeper.sh,resources=*,verbs=get;list;watch;create;update;patch;delete + +// Reconcile reads that state of the cluster for a constraint object and makes changes based on the state read +// and what is in the constraint.Spec. +func (r *ReconcileConfigStatus) Reconcile(ctx context.Context, request reconcile.Request) (reconcile.Result, error) { + et := &configv1alpha1.Config{} + err := r.reader.Get(ctx, request.NamespacedName, et) + if err != nil { + // If the Config does not exist then we are done + if errors.IsNotFound(err) { + return reconcile.Result{}, nil + } + return reconcile.Result{}, err + } + + sObjs := &v1beta1.ConfigPodStatusList{} + if err := r.reader.List( + ctx, + sObjs, + client.MatchingLabels{v1beta1.ConfigNameLabel: request.Name}, + client.InNamespace(util.GetNamespace()), + ); err != nil { + return reconcile.Result{}, err + } + statusObjs := make(sortableStatuses, len(sObjs.Items)) + copy(statusObjs, sObjs.Items) + sort.Sort(statusObjs) + + var s []v1beta1.ConfigPodStatusStatus + // created is true if at least one Pod hasn't reported any errors + + for i := range statusObjs { + // Don't report status if it's not for the correct object. This can happen + // if a watch gets interrupted, causing the constraint status to be deleted + // out from underneath it + if statusObjs[i].Status.ConfigUID != et.GetUID() { + continue + } + s = append(s, statusObjs[i].Status) + } + + et.Status.ByPod = s + + if err := r.statusClient.Status().Update(ctx, et); err != nil { + return reconcile.Result{Requeue: true}, nil + } + return reconcile.Result{}, nil +} + +type sortableStatuses []v1beta1.ConfigPodStatus + +func (s sortableStatuses) Len() int { + return len(s) +} + +func (s sortableStatuses) Less(i, j int) bool { + return s[i].Status.ID < s[j].Status.ID +} + +func (s sortableStatuses) Swap(i, j int) { + s[i], s[j] = s[j], s[i] +} From 9c27e838762d4c8df4debb865e4c26bdc575543d Mon Sep 17 00:00:00 2001 From: Avinash Patnala Date: Tue, 17 Sep 2024 21:55:48 +0000 Subject: [PATCH 04/28] implement config pod status logic Signed-off-by: Avinash Patnala --- pkg/controller/config/config_controller.go | 80 +++++++++++++++++++++- 1 file changed, 77 insertions(+), 3 deletions(-) diff --git a/pkg/controller/config/config_controller.go b/pkg/controller/config/config_controller.go index 49bb06963a6..576014949bd 100644 --- a/pkg/controller/config/config_controller.go +++ b/pkg/controller/config/config_controller.go @@ -20,16 +20,19 @@ import ( "fmt" configv1alpha1 "github.com/open-policy-agent/gatekeeper/v3/apis/config/v1alpha1" + statusv1beta1 "github.com/open-policy-agent/gatekeeper/v3/apis/status/v1beta1" cm "github.com/open-policy-agent/gatekeeper/v3/pkg/cachemanager" "github.com/open-policy-agent/gatekeeper/v3/pkg/cachemanager/aggregator" "github.com/open-policy-agent/gatekeeper/v3/pkg/controller/config/process" "github.com/open-policy-agent/gatekeeper/v3/pkg/keys" "github.com/open-policy-agent/gatekeeper/v3/pkg/readiness" + "github.com/open-policy-agent/gatekeeper/v3/pkg/util" "github.com/open-policy-agent/gatekeeper/v3/pkg/watch" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/errors" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller" "sigs.k8s.io/controller-runtime/pkg/handler" @@ -164,7 +167,7 @@ func (r *ReconcileConfig) Reconcile(ctx context.Context, request reconcile.Reque err := r.reader.Get(ctx, request.NamespacedName, instance) if err != nil { // if config is not found, we should remove cached data - if errors.IsNotFound(err) { + if apierrors.IsNotFound(err) { exists = false } else { // Error reading the object - requeue the request. @@ -177,7 +180,14 @@ func (r *ReconcileConfig) Reconcile(ctx context.Context, request reconcile.Reque // If the config is being deleted the user is saying they don't want to // sync anything gvksToSync := []schema.GroupVersionKind{} - if exists && instance.GetDeletionTimestamp().IsZero() { + + // deleted is true if the instance doesn't exist or has a deletion timestamp. + deleted := !exists || !instance.GetDeletionTimestamp().IsZero() + + if deleted { + // Delete config pod status if config is deleted. + r.deleteStatus(ctx, instance.GetName()) + } else { for _, entry := range instance.Spec.Sync.SyncOnly { gvk := schema.GroupVersionKind{Group: entry.Group, Version: entry.Version, Kind: entry.Kind} gvksToSync = append(gvksToSync, gvk) @@ -185,6 +195,7 @@ func (r *ReconcileConfig) Reconcile(ctx context.Context, request reconcile.Reque newExcluder.Add(instance.Spec.Match) statsEnabled = instance.Spec.Readiness.StatsEnabled + r.updateOrCreatePodStatus(ctx, instance) } // Enable verbose readiness stats if requested. @@ -207,3 +218,66 @@ func (r *ReconcileConfig) Reconcile(ctx context.Context, request reconcile.Reque r.tracker.For(configGVK).Observe(instance) return reconcile.Result{}, nil } + +func (r *ReconcileConfig) deleteStatus(ctx context.Context, cfgName string) error { + status := &statusv1beta1.ConfigPodStatus{} + pod, err := r.getPod(ctx) + if err != nil { + return fmt.Errorf("getting reconciler pod: %w", err) + } + sName, err := statusv1beta1.KeyForConfig(pod.Name, cfgName) + if err != nil { + return fmt.Errorf("getting key for config: %w", err) + } + status.SetName(sName) + status.SetNamespace(util.GetNamespace()) + if err := r.writer.Delete(ctx, status); err != nil && !apierrors.IsNotFound(err) { + return err + } + return nil +} + +func (r *ReconcileConfig) updateOrCreatePodStatus(ctx context.Context, cfg *configv1alpha1.Config) error { + pod, err := r.getPod(ctx) + if err != nil { + return fmt.Errorf("getting reconciler pod: %w", err) + } + + // Check if it exists already + sNS := pod.Namespace + sName, err := statusv1beta1.KeyForConfig(pod.Name, cfg.GetName()) + if err != nil { + return fmt.Errorf("getting key for config: %w", err) + } + shouldCreate := true + status := &statusv1beta1.ConfigPodStatus{} + + err = r.reader.Get(ctx, types.NamespacedName{Namespace: sNS, Name: sName}, status) + switch { + case err == nil: + shouldCreate = false + case apierrors.IsNotFound(err): + if status, err = r.newConfigStatus(pod, cfg); err != nil { + return fmt.Errorf("creating new config status: %w", err) + } + default: + return fmt.Errorf("getting config status in name %s, namespace %s: %w", cfg.GetName(), cfg.GetNamespace(), err) + } + + status.Status.ObservedGeneration = cfg.GetGeneration() + + if shouldCreate { + return r.writer.Create(ctx, status) + } + return r.writer.Update(ctx, status) +} + +func (r *ReconcileConfig) newConfigStatus(pod *corev1.Pod, cfg *configv1alpha1.Config) (*statusv1beta1.ConfigPodStatus, error) { + status, err := statusv1beta1.NewConfigStatusForPod(pod, cfg.GetName(), r.scheme) + if err != nil { + return nil, fmt.Errorf("creating status for pod: %w", err) + } + status.Status.ConfigUID = cfg.GetUID() + + return status, nil +} From 290b46e7d1805ac3f75b4336f7e08b341781cc41 Mon Sep 17 00:00:00 2001 From: Avinash Patnala Date: Tue, 17 Sep 2024 22:01:05 +0000 Subject: [PATCH 05/28] remove make comment from api docs Signed-off-by: Avinash Patnala --- apis/status/v1beta1/configpodstatus_types.go | 1 - config/crd/bases/config.gatekeeper.sh_configs.yaml | 2 -- config/crd/bases/status.gatekeeper.sh_configpodstatuses.yaml | 2 -- .../charts/gatekeeper/crds/config-customresourcedefinition.yaml | 1 - .../crds/configpodstatus-customresourcedefinition.yaml | 1 - manifest_staging/deploy/gatekeeper.yaml | 2 -- 6 files changed, 9 deletions(-) diff --git a/apis/status/v1beta1/configpodstatus_types.go b/apis/status/v1beta1/configpodstatus_types.go index 6160355ab66..46cdc955dae 100644 --- a/apis/status/v1beta1/configpodstatus_types.go +++ b/apis/status/v1beta1/configpodstatus_types.go @@ -15,7 +15,6 @@ import ( // +kubebuilder:object:generate=true type ConfigPodStatusStatus struct { - // Important: Run "make" to regenerate code after modifying this file ID string `json:"id,omitempty"` ConfigUID types.UID `json:"configUID,omitempty"` Operations []string `json:"operations,omitempty"` diff --git a/config/crd/bases/config.gatekeeper.sh_configs.yaml b/config/crd/bases/config.gatekeeper.sh_configs.yaml index 2bed80a7eb0..ae9ed3fd806 100644 --- a/config/crd/bases/config.gatekeeper.sh_configs.yaml +++ b/config/crd/bases/config.gatekeeper.sh_configs.yaml @@ -134,8 +134,6 @@ spec: type: object type: array id: - description: 'Important: Run "make" to regenerate code after - modifying this file' type: string observedGeneration: format: int64 diff --git a/config/crd/bases/status.gatekeeper.sh_configpodstatuses.yaml b/config/crd/bases/status.gatekeeper.sh_configpodstatuses.yaml index 57604c6083e..1afb74b2ece 100644 --- a/config/crd/bases/status.gatekeeper.sh_configpodstatuses.yaml +++ b/config/crd/bases/status.gatekeeper.sh_configpodstatuses.yaml @@ -55,8 +55,6 @@ spec: type: object type: array id: - description: 'Important: Run "make" to regenerate code after modifying - this file' type: string observedGeneration: format: int64 diff --git a/manifest_staging/charts/gatekeeper/crds/config-customresourcedefinition.yaml b/manifest_staging/charts/gatekeeper/crds/config-customresourcedefinition.yaml index 5182e6625a2..8a5afdeb640 100644 --- a/manifest_staging/charts/gatekeeper/crds/config-customresourcedefinition.yaml +++ b/manifest_staging/charts/gatekeeper/crds/config-customresourcedefinition.yaml @@ -134,7 +134,6 @@ spec: type: object type: array id: - description: 'Important: Run "make" to regenerate code after modifying this file' type: string observedGeneration: format: int64 diff --git a/manifest_staging/charts/gatekeeper/crds/configpodstatus-customresourcedefinition.yaml b/manifest_staging/charts/gatekeeper/crds/configpodstatus-customresourcedefinition.yaml index 750eb39e62b..932846d8a60 100644 --- a/manifest_staging/charts/gatekeeper/crds/configpodstatus-customresourcedefinition.yaml +++ b/manifest_staging/charts/gatekeeper/crds/configpodstatus-customresourcedefinition.yaml @@ -62,7 +62,6 @@ spec: type: object type: array id: - description: 'Important: Run "make" to regenerate code after modifying this file' type: string observedGeneration: format: int64 diff --git a/manifest_staging/deploy/gatekeeper.yaml b/manifest_staging/deploy/gatekeeper.yaml index 575f701c928..8d83849cf93 100644 --- a/manifest_staging/deploy/gatekeeper.yaml +++ b/manifest_staging/deploy/gatekeeper.yaml @@ -2444,7 +2444,6 @@ spec: type: object type: array id: - description: 'Important: Run "make" to regenerate code after modifying this file' type: string observedGeneration: format: int64 @@ -2593,7 +2592,6 @@ spec: type: object type: array id: - description: 'Important: Run "make" to regenerate code after modifying this file' type: string observedGeneration: format: int64 From 2d6ccba6950e44f4224f78ec42cfbe38ca80ec90 Mon Sep 17 00:00:00 2001 From: Avinash Patnala Date: Tue, 17 Sep 2024 22:18:12 +0000 Subject: [PATCH 06/28] fix go lint errors Signed-off-by: Avinash Patnala --- pkg/controller/config/config_controller.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/pkg/controller/config/config_controller.go b/pkg/controller/config/config_controller.go index 576014949bd..7e17298468b 100644 --- a/pkg/controller/config/config_controller.go +++ b/pkg/controller/config/config_controller.go @@ -186,7 +186,9 @@ func (r *ReconcileConfig) Reconcile(ctx context.Context, request reconcile.Reque if deleted { // Delete config pod status if config is deleted. - r.deleteStatus(ctx, instance.GetName()) + if err := r.deleteStatus(ctx, instance.GetName()); err != nil { + return reconcile.Result{}, fmt.Errorf("config-controller: error deleting config pod status: %w", err) + } } else { for _, entry := range instance.Spec.Sync.SyncOnly { gvk := schema.GroupVersionKind{Group: entry.Group, Version: entry.Version, Kind: entry.Kind} @@ -195,7 +197,9 @@ func (r *ReconcileConfig) Reconcile(ctx context.Context, request reconcile.Reque newExcluder.Add(instance.Spec.Match) statsEnabled = instance.Spec.Readiness.StatsEnabled - r.updateOrCreatePodStatus(ctx, instance) + if err := r.updateOrCreatePodStatus(ctx, instance); err != nil { + return reconcile.Result{}, fmt.Errorf("config-controller: error creating config pod status: %w", err) + } } // Enable verbose readiness stats if requested. From b149d3f02cdce9169a5789ab22eab04b66d0746f Mon Sep 17 00:00:00 2001 From: Avinash Patnala Date: Tue, 17 Sep 2024 22:57:27 +0000 Subject: [PATCH 07/28] fix helm charts Signed-off-by: Avinash Patnala --- cmd/build/helmify/kustomization.yaml | 6 ++++++ .../crds/configpodstatus-customresourcedefinition.yaml | 4 ---- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/cmd/build/helmify/kustomization.yaml b/cmd/build/helmify/kustomization.yaml index 1c1943f2874..45eee7b3db5 100644 --- a/cmd/build/helmify/kustomization.yaml +++ b/cmd/build/helmify/kustomization.yaml @@ -40,6 +40,12 @@ patchesJson6902: kind: CustomResourceDefinition name: expansiontemplatepodstatuses.status.gatekeeper.sh path: labels_patch.yaml + - target: + group: apiextensions.k8s.io + version: v1 + kind: CustomResourceDefinition + name: configpodstatuses.status.gatekeeper.sh + path: labels_patch.yaml - target: group: apiextensions.k8s.io version: v1 diff --git a/manifest_staging/charts/gatekeeper/crds/configpodstatus-customresourcedefinition.yaml b/manifest_staging/charts/gatekeeper/crds/configpodstatus-customresourcedefinition.yaml index 932846d8a60..f351b718375 100644 --- a/manifest_staging/charts/gatekeeper/crds/configpodstatus-customresourcedefinition.yaml +++ b/manifest_staging/charts/gatekeeper/crds/configpodstatus-customresourcedefinition.yaml @@ -5,11 +5,7 @@ metadata: annotations: controller-gen.kubebuilder.io/version: v0.14.0 labels: - app: '{{ template "gatekeeper.name" . }}' - chart: '{{ template "gatekeeper.name" . }}' gatekeeper.sh/system: "yes" - heritage: '{{ .Release.Service }}' - release: '{{ .Release.Name }}' name: configpodstatuses.status.gatekeeper.sh spec: group: status.gatekeeper.sh From bc5ba07c0ccf46dec0c571e20c7fc60484899110 Mon Sep 17 00:00:00 2001 From: Avinash Patnala Date: Tue, 17 Sep 2024 23:31:20 +0000 Subject: [PATCH 08/28] fix double import Signed-off-by: Avinash Patnala --- pkg/controller/config/config_controller_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/controller/config/config_controller_test.go b/pkg/controller/config/config_controller_test.go index a44b6462436..9f6868fca52 100644 --- a/pkg/controller/config/config_controller_test.go +++ b/pkg/controller/config/config_controller_test.go @@ -40,7 +40,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.org/x/net/context" - corev1 "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -159,7 +159,7 @@ func TestReconcile(t *testing.T) { fakes.WithName("no-pod"), ) - rec, err := newReconciler(mgr, cacheManager, cs, tracker, func(context.Context) (*corev1.Pod, error) { return pod, nil }) + rec, err := newReconciler(mgr, cacheManager, cs, tracker, func(context.Context) (*v1.Pod, error) { return pod, nil }) require.NoError(t, err) // Wrap the Controller Reconcile function so it writes each request to a map when it is finished reconciling. @@ -342,8 +342,8 @@ func TestConfig_DeleteSyncResources(t *testing.T) { fakes.WithNamespace("default"), fakes.WithName("testpod"), ) - pod.Spec = corev1.PodSpec{ - Containers: []corev1.Container{ + pod.Spec = v1.PodSpec{ + Containers: []v1.Container{ { Name: "nginx", Image: "nginx", @@ -454,7 +454,7 @@ func setupController(ctx context.Context, mgr manager.Manager, wm *watch.Manager fakes.WithName("no-pod"), ) - rec, err := newReconciler(mgr, cacheManager, cs, tracker, func(context.Context) (*corev1.Pod, error) { return pod, nil }) + rec, err := newReconciler(mgr, cacheManager, cs, tracker, func(context.Context) (*v1.Pod, error) { return pod, nil }) if err != nil { return nil, fmt.Errorf("creating reconciler: %w", err) } @@ -633,7 +633,7 @@ func TestConfig_Retries(t *testing.T) { fakes.WithName("no-pod"), ) - rec, _ := newReconciler(mgr, cacheManager, cs, tracker, func(context.Context) (*corev1.Pod, error) { return pod, nil }) + rec, _ := newReconciler(mgr, cacheManager, cs, tracker, func(context.Context) (*v1.Pod, error) { return pod, nil }) err = add(mgr, rec) if err != nil { t.Fatal(err) From 88065029248a40ca31ffa1287bf331f3849b269b Mon Sep 17 00:00:00 2001 From: Avinash Patnala Date: Wed, 18 Sep 2024 18:31:51 +0000 Subject: [PATCH 09/28] add or delete pod status at the end Signed-off-by: Avinash Patnala --- pkg/controller/config/config_controller.go | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/pkg/controller/config/config_controller.go b/pkg/controller/config/config_controller.go index 7e17298468b..3d39476480e 100644 --- a/pkg/controller/config/config_controller.go +++ b/pkg/controller/config/config_controller.go @@ -184,12 +184,7 @@ func (r *ReconcileConfig) Reconcile(ctx context.Context, request reconcile.Reque // deleted is true if the instance doesn't exist or has a deletion timestamp. deleted := !exists || !instance.GetDeletionTimestamp().IsZero() - if deleted { - // Delete config pod status if config is deleted. - if err := r.deleteStatus(ctx, instance.GetName()); err != nil { - return reconcile.Result{}, fmt.Errorf("config-controller: error deleting config pod status: %w", err) - } - } else { + if !deleted { for _, entry := range instance.Spec.Sync.SyncOnly { gvk := schema.GroupVersionKind{Group: entry.Group, Version: entry.Version, Kind: entry.Kind} gvksToSync = append(gvksToSync, gvk) @@ -197,9 +192,6 @@ func (r *ReconcileConfig) Reconcile(ctx context.Context, request reconcile.Reque newExcluder.Add(instance.Spec.Match) statsEnabled = instance.Spec.Readiness.StatsEnabled - if err := r.updateOrCreatePodStatus(ctx, instance); err != nil { - return reconcile.Result{}, fmt.Errorf("config-controller: error creating config pod status: %w", err) - } } // Enable verbose readiness stats if requested. @@ -220,7 +212,11 @@ func (r *ReconcileConfig) Reconcile(ctx context.Context, request reconcile.Reque } r.tracker.For(configGVK).Observe(instance) - return reconcile.Result{}, nil + + if deleted { + return reconcile.Result{}, r.deleteStatus(ctx, instance.GetName()) + } + return reconcile.Result{}, r.updateOrCreatePodStatus(ctx, instance) } func (r *ReconcileConfig) deleteStatus(ctx context.Context, cfgName string) error { From a2f6e9e8e1b5453e0ef46e8a889b9a4f9cea73d2 Mon Sep 17 00:00:00 2001 From: Avinash Patnala Date: Wed, 18 Sep 2024 19:05:28 +0000 Subject: [PATCH 10/28] pass request.name as key when config is deleted Signed-off-by: Avinash Patnala --- pkg/controller/config/config_controller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/controller/config/config_controller.go b/pkg/controller/config/config_controller.go index 3d39476480e..2bb0ea5d198 100644 --- a/pkg/controller/config/config_controller.go +++ b/pkg/controller/config/config_controller.go @@ -214,7 +214,7 @@ func (r *ReconcileConfig) Reconcile(ctx context.Context, request reconcile.Reque r.tracker.For(configGVK).Observe(instance) if deleted { - return reconcile.Result{}, r.deleteStatus(ctx, instance.GetName()) + return reconcile.Result{}, r.deleteStatus(ctx, request.Name) } return reconcile.Result{}, r.updateOrCreatePodStatus(ctx, instance) } From 9cf8fe5fd3d1f5d0ed900d1f4b8e78396871d145 Mon Sep 17 00:00:00 2001 From: Avinash Patnala Date: Wed, 18 Sep 2024 20:16:07 +0000 Subject: [PATCH 11/28] update config controller to pass request.NameSpace.Name Signed-off-by: Avinash Patnala --- pkg/controller/config/config_controller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/controller/config/config_controller.go b/pkg/controller/config/config_controller.go index 2bb0ea5d198..da1717292af 100644 --- a/pkg/controller/config/config_controller.go +++ b/pkg/controller/config/config_controller.go @@ -214,7 +214,7 @@ func (r *ReconcileConfig) Reconcile(ctx context.Context, request reconcile.Reque r.tracker.For(configGVK).Observe(instance) if deleted { - return reconcile.Result{}, r.deleteStatus(ctx, request.Name) + return reconcile.Result{}, r.deleteStatus(ctx, request.NamespacedName.Name) } return reconcile.Result{}, r.updateOrCreatePodStatus(ctx, instance) } From 00c3e41b972beaee5685e6a70cbabcb0d0692ec2 Mon Sep 17 00:00:00 2001 From: Avinash Patnala Date: Thu, 19 Sep 2024 01:40:54 +0000 Subject: [PATCH 12/28] wrap fake client with mutex logic to resolve data race issues Signed-off-by: Avinash Patnala --- pkg/readiness/ready_tracker.go | 11 +++++++++++ pkg/readiness/ready_tracker_unit_test.go | 4 +++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/pkg/readiness/ready_tracker.go b/pkg/readiness/ready_tracker.go index dffec0903e7..6ff2a9bf035 100644 --- a/pkg/readiness/ready_tracker.go +++ b/pkg/readiness/ready_tracker.go @@ -89,6 +89,17 @@ type Tracker struct { trackListerPredicateOverride retryPredicate } +type WrapFakeClientWithMutex struct { + listMutex sync.Mutex + fakeLister Lister +} + +func (w *WrapFakeClientWithMutex) List(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error { + w.listMutex.Lock() + defer w.listMutex.Unlock() + return w.fakeLister.List(ctx, list, opts...) +} + // NewTracker creates a new Tracker and initializes the internal trackers. func NewTracker(lister Lister, mutationEnabled, externalDataEnabled, expansionEnabled bool) *Tracker { // TODO: Dereference crashOnFailureFetchingExpectations when we change crashOnFailureFetchingExpectations to a flag diff --git a/pkg/readiness/ready_tracker_unit_test.go b/pkg/readiness/ready_tracker_unit_test.go index ba5d039b941..74baa906704 100644 --- a/pkg/readiness/ready_tracker_unit_test.go +++ b/pkg/readiness/ready_tracker_unit_test.go @@ -211,7 +211,9 @@ func Test_ReadyTracker_TryCancelTemplate_No_Retries(t *testing.T) { // Verify that TryCancelTemplate must be called enough times to remove all retries before canceling a template. func Test_ReadyTracker_TryCancelTemplate_Retries(t *testing.T) { lister := fake.NewClientBuilder().WithScheme(mustInitializeScheme(runtime.NewScheme())).WithRuntimeObjects(convertedTemplate.DeepCopyObject()).Build() - rt := newTracker(lister, false, false, false, false, nil, func() objData { + wrapLister := WrapFakeClientWithMutex{fakeLister: lister} + + rt := newTracker(&wrapLister, false, false, false, false, nil, func() objData { return objData{retries: 2} }) From 842b26a299d1bc2d023520982041c466b62e676b Mon Sep 17 00:00:00 2001 From: Avinash Patnala Date: Thu, 19 Sep 2024 01:52:05 +0000 Subject: [PATCH 13/28] use wrapped fake client in failing test Signed-off-by: Avinash Patnala --- pkg/readiness/ready_tracker_unit_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/pkg/readiness/ready_tracker_unit_test.go b/pkg/readiness/ready_tracker_unit_test.go index 74baa906704..f99ba6007bb 100644 --- a/pkg/readiness/ready_tracker_unit_test.go +++ b/pkg/readiness/ready_tracker_unit_test.go @@ -171,8 +171,10 @@ func mustInitializeScheme(scheme *runtime.Scheme) *runtime.Scheme { // Verify that TryCancelTemplate functions the same as regular CancelTemplate if readinessRetries is set to 0. func Test_ReadyTracker_TryCancelTemplate_No_Retries(t *testing.T) { lister := fake.NewClientBuilder().WithScheme(mustInitializeScheme(runtime.NewScheme())).WithRuntimeObjects(convertedTemplate.DeepCopyObject()).Build() - rt := newTracker(lister, false, false, false, false, nil, func() objData { - return objData{retries: 0} + wrapLister := WrapFakeClientWithMutex{fakeLister: lister} + + rt := newTracker(&wrapLister, false, false, false, false, nil, func() objData { + return objData{retries: 2} }) // Run kicks off all the tracking From 9d05fc2d46a5c6574cd7ce3df80b10f458da8f84 Mon Sep 17 00:00:00 2001 From: Avinash Patnala Date: Thu, 19 Sep 2024 17:43:15 +0000 Subject: [PATCH 14/28] fix typo in retry count Signed-off-by: Avinash Patnala --- pkg/controller/config/config_controller_test.go | 1 - pkg/readiness/ready_tracker_unit_test.go | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/controller/config/config_controller_test.go b/pkg/controller/config/config_controller_test.go index 9f6868fca52..817e34e10b0 100644 --- a/pkg/controller/config/config_controller_test.go +++ b/pkg/controller/config/config_controller_test.go @@ -40,7 +40,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.org/x/net/context" - v1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" diff --git a/pkg/readiness/ready_tracker_unit_test.go b/pkg/readiness/ready_tracker_unit_test.go index f99ba6007bb..c443a1e3e9a 100644 --- a/pkg/readiness/ready_tracker_unit_test.go +++ b/pkg/readiness/ready_tracker_unit_test.go @@ -174,7 +174,7 @@ func Test_ReadyTracker_TryCancelTemplate_No_Retries(t *testing.T) { wrapLister := WrapFakeClientWithMutex{fakeLister: lister} rt := newTracker(&wrapLister, false, false, false, false, nil, func() objData { - return objData{retries: 2} + return objData{retries: 0} }) // Run kicks off all the tracking From 5f220bb5cd117809fbd524211d49ee45eeb81c29 Mon Sep 17 00:00:00 2001 From: Avinash Patnala Date: Fri, 20 Sep 2024 22:58:46 +0000 Subject: [PATCH 15/28] fix config by pod status not updating issue Signed-off-by: Avinash Patnala --- pkg/controller/config/config_controller.go | 7 +++++++ .../configstatus/configstatus_controller.go | 14 +++++++------- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/pkg/controller/config/config_controller.go b/pkg/controller/config/config_controller.go index da1717292af..f5bfd5280fa 100644 --- a/pkg/controller/config/config_controller.go +++ b/pkg/controller/config/config_controller.go @@ -24,6 +24,7 @@ import ( cm "github.com/open-policy-agent/gatekeeper/v3/pkg/cachemanager" "github.com/open-policy-agent/gatekeeper/v3/pkg/cachemanager/aggregator" "github.com/open-policy-agent/gatekeeper/v3/pkg/controller/config/process" + "github.com/open-policy-agent/gatekeeper/v3/pkg/controller/configstatus" "github.com/open-policy-agent/gatekeeper/v3/pkg/keys" "github.com/open-policy-agent/gatekeeper/v3/pkg/readiness" "github.com/open-policy-agent/gatekeeper/v3/pkg/util" @@ -118,6 +119,12 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error { return err } + err = c.Watch( + source.Kind(mgr.GetCache(), &statusv1beta1.ConfigPodStatus{}, handler.TypedEnqueueRequestsFromMapFunc(configstatus.PodStatusToConfigMapper(true, util.EventPackerMapFunc())))) + if err != nil { + return err + } + return nil } diff --git a/pkg/controller/configstatus/configstatus_controller.go b/pkg/controller/configstatus/configstatus_controller.go index 7f91cb80cbb..7825286b5cd 100644 --- a/pkg/controller/configstatus/configstatus_controller.go +++ b/pkg/controller/configstatus/configstatus_controller.go @@ -69,10 +69,12 @@ func newReconciler(mgr manager.Manager) reconcile.Reconciler { } } +type PackerMap func(obj client.Object) []reconcile.Request + // PodStatusToConfigMapper correlates a ConfigPodStatus with its corresponding Config. // `selfOnly` tells the mapper to only map statuses corresponding to the current pod. -func PodStatusToConfigMapper(selfOnly bool) handler.TypedMapFunc[*v1beta1.ConfigPodStatus] { - return func(_ context.Context, obj *v1beta1.ConfigPodStatus) []reconcile.Request { +func PodStatusToConfigMapper(selfOnly bool, packerMap handler.MapFunc) handler.TypedMapFunc[*v1beta1.ConfigPodStatus] { + return func(ctx context.Context, obj *v1beta1.ConfigPodStatus) []reconcile.Request { labels := obj.GetLabels() name, ok := labels[v1beta1.ConfigNameLabel] if !ok { @@ -101,16 +103,14 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error { return err } - // Watch for changes to ExpansionTemplateStatus + // Watch for changes to ConfigStatus err = c.Watch( - source.Kind(mgr.GetCache(), &v1beta1.ConfigPodStatus{}, - handler.TypedEnqueueRequestsFromMapFunc(PodStatusToConfigMapper(false)), - )) + source.Kind(mgr.GetCache(), &v1beta1.ConfigPodStatus{}, handler.TypedEnqueueRequestsFromMapFunc(PodStatusToConfigMapper(false, util.EventPackerMapFunc())))) if err != nil { return err } - // Watch for changes to ExpansionTemplate + // Watch for changes to Config err = c.Watch(source.Kind(mgr.GetCache(), &configv1alpha1.Config{}, &handler.TypedEnqueueRequestForObject[*configv1alpha1.Config]{})) if err != nil { return err From 0f431eb04632a92a586aa8e9f6a90265573968ae Mon Sep 17 00:00:00 2001 From: Avinash Patnala Date: Fri, 20 Sep 2024 23:19:11 +0000 Subject: [PATCH 16/28] Address review comments Signed-off-by: Avinash Patnala --- pkg/controller/config/config_controller.go | 2 +- .../configstatus/configstatus_controller.go | 12 +++++++----- pkg/readiness/ready_tracker_unit_test.go | 11 +++++++++++ 3 files changed, 19 insertions(+), 6 deletions(-) diff --git a/pkg/controller/config/config_controller.go b/pkg/controller/config/config_controller.go index f5bfd5280fa..78badceecb7 100644 --- a/pkg/controller/config/config_controller.go +++ b/pkg/controller/config/config_controller.go @@ -188,7 +188,7 @@ func (r *ReconcileConfig) Reconcile(ctx context.Context, request reconcile.Reque // sync anything gvksToSync := []schema.GroupVersionKind{} - // deleted is true if the instance doesn't exist or has a deletion timestamp. + // K8s API conventions consider an object to be deleted when either the object no longer exists or when a deletion timestamp has been set. deleted := !exists || !instance.GetDeletionTimestamp().IsZero() if !deleted { diff --git a/pkg/controller/configstatus/configstatus_controller.go b/pkg/controller/configstatus/configstatus_controller.go index 7825286b5cd..2d272b845ef 100644 --- a/pkg/controller/configstatus/configstatus_controller.go +++ b/pkg/controller/configstatus/configstatus_controller.go @@ -24,6 +24,7 @@ import ( configv1alpha1 "github.com/open-policy-agent/gatekeeper/v3/apis/config/v1alpha1" "github.com/open-policy-agent/gatekeeper/v3/apis/status/v1beta1" "github.com/open-policy-agent/gatekeeper/v3/pkg/logging" + "github.com/open-policy-agent/gatekeeper/v3/pkg/operations" "github.com/open-policy-agent/gatekeeper/v3/pkg/readiness" "github.com/open-policy-agent/gatekeeper/v3/pkg/util" "github.com/open-policy-agent/gatekeeper/v3/pkg/watch" @@ -52,6 +53,9 @@ func (a *Adder) InjectTracker(_ *readiness.Tracker) {} // Add creates a new config Status Controller and adds it to the Manager. The Manager will set fields on the Controller // and Start it when the Manager is Started. func (a *Adder) Add(mgr manager.Manager) error { + if !operations.IsAssigned(operations.Status) { + return nil + } r := newReconciler(mgr) return add(mgr, r) } @@ -74,7 +78,7 @@ type PackerMap func(obj client.Object) []reconcile.Request // PodStatusToConfigMapper correlates a ConfigPodStatus with its corresponding Config. // `selfOnly` tells the mapper to only map statuses corresponding to the current pod. func PodStatusToConfigMapper(selfOnly bool, packerMap handler.MapFunc) handler.TypedMapFunc[*v1beta1.ConfigPodStatus] { - return func(ctx context.Context, obj *v1beta1.ConfigPodStatus) []reconcile.Request { + return func(_ context.Context, obj *v1beta1.ConfigPodStatus) []reconcile.Request { labels := obj.GetLabels() name, ok := labels[v1beta1.ConfigNameLabel] if !ok { @@ -95,7 +99,8 @@ func PodStatusToConfigMapper(selfOnly bool, packerMap handler.MapFunc) handler.T } } -// add adds a new Controller to mgr with r as the reconcile.Reconciler. +// Add creates a new config status Controller and adds it to the Manager. The Manager will set fields on the Controller +// and Start it when the Manager is Started. func add(mgr manager.Manager, r reconcile.Reconciler) error { // Create a new controller c, err := controller.New("config-status-controller", mgr, controller.Options{Reconciler: r}) @@ -103,14 +108,12 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error { return err } - // Watch for changes to ConfigStatus err = c.Watch( source.Kind(mgr.GetCache(), &v1beta1.ConfigPodStatus{}, handler.TypedEnqueueRequestsFromMapFunc(PodStatusToConfigMapper(false, util.EventPackerMapFunc())))) if err != nil { return err } - // Watch for changes to Config err = c.Watch(source.Kind(mgr.GetCache(), &configv1alpha1.Config{}, &handler.TypedEnqueueRequestForObject[*configv1alpha1.Config]{})) if err != nil { return err @@ -160,7 +163,6 @@ func (r *ReconcileConfigStatus) Reconcile(ctx context.Context, request reconcile sort.Sort(statusObjs) var s []v1beta1.ConfigPodStatusStatus - // created is true if at least one Pod hasn't reported any errors for i := range statusObjs { // Don't report status if it's not for the correct object. This can happen diff --git a/pkg/readiness/ready_tracker_unit_test.go b/pkg/readiness/ready_tracker_unit_test.go index c443a1e3e9a..d2adf8c9ab0 100644 --- a/pkg/readiness/ready_tracker_unit_test.go +++ b/pkg/readiness/ready_tracker_unit_test.go @@ -49,6 +49,17 @@ const ( tick = 1 * time.Second ) +type WrapFakeClientWithMutex struct { + listMutex sync.Mutex + fakeLister Lister +} + +func (w *WrapFakeClientWithMutex) List(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error { + w.listMutex.Lock() + defer w.listMutex.Unlock() + return w.fakeLister.List(ctx, list, opts...) +} + var ( testConstraintTemplate = templates.ConstraintTemplate{ ObjectMeta: v1.ObjectMeta{ From 79c048b893765bcf640a948a94f9101e8b44d3ff Mon Sep 17 00:00:00 2001 From: Avinash Patnala Date: Fri, 20 Sep 2024 23:32:12 +0000 Subject: [PATCH 17/28] fix lint errors Signed-off-by: Avinash Patnala --- pkg/controller/config/config_controller.go | 2 +- .../configstatus/configstatus_controller.go | 6 ++---- pkg/readiness/ready_tracker.go | 11 ----------- 3 files changed, 3 insertions(+), 16 deletions(-) diff --git a/pkg/controller/config/config_controller.go b/pkg/controller/config/config_controller.go index 78badceecb7..b675f502741 100644 --- a/pkg/controller/config/config_controller.go +++ b/pkg/controller/config/config_controller.go @@ -120,7 +120,7 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error { } err = c.Watch( - source.Kind(mgr.GetCache(), &statusv1beta1.ConfigPodStatus{}, handler.TypedEnqueueRequestsFromMapFunc(configstatus.PodStatusToConfigMapper(true, util.EventPackerMapFunc())))) + source.Kind(mgr.GetCache(), &statusv1beta1.ConfigPodStatus{}, handler.TypedEnqueueRequestsFromMapFunc(configstatus.PodStatusToConfigMapper(true)))) if err != nil { return err } diff --git a/pkg/controller/configstatus/configstatus_controller.go b/pkg/controller/configstatus/configstatus_controller.go index 2d272b845ef..99bd124d897 100644 --- a/pkg/controller/configstatus/configstatus_controller.go +++ b/pkg/controller/configstatus/configstatus_controller.go @@ -73,11 +73,9 @@ func newReconciler(mgr manager.Manager) reconcile.Reconciler { } } -type PackerMap func(obj client.Object) []reconcile.Request - // PodStatusToConfigMapper correlates a ConfigPodStatus with its corresponding Config. // `selfOnly` tells the mapper to only map statuses corresponding to the current pod. -func PodStatusToConfigMapper(selfOnly bool, packerMap handler.MapFunc) handler.TypedMapFunc[*v1beta1.ConfigPodStatus] { +func PodStatusToConfigMapper(selfOnly bool) handler.TypedMapFunc[*v1beta1.ConfigPodStatus] { return func(_ context.Context, obj *v1beta1.ConfigPodStatus) []reconcile.Request { labels := obj.GetLabels() name, ok := labels[v1beta1.ConfigNameLabel] @@ -109,7 +107,7 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error { } err = c.Watch( - source.Kind(mgr.GetCache(), &v1beta1.ConfigPodStatus{}, handler.TypedEnqueueRequestsFromMapFunc(PodStatusToConfigMapper(false, util.EventPackerMapFunc())))) + source.Kind(mgr.GetCache(), &v1beta1.ConfigPodStatus{}, handler.TypedEnqueueRequestsFromMapFunc(PodStatusToConfigMapper(false)))) if err != nil { return err } diff --git a/pkg/readiness/ready_tracker.go b/pkg/readiness/ready_tracker.go index 6ff2a9bf035..dffec0903e7 100644 --- a/pkg/readiness/ready_tracker.go +++ b/pkg/readiness/ready_tracker.go @@ -89,17 +89,6 @@ type Tracker struct { trackListerPredicateOverride retryPredicate } -type WrapFakeClientWithMutex struct { - listMutex sync.Mutex - fakeLister Lister -} - -func (w *WrapFakeClientWithMutex) List(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error { - w.listMutex.Lock() - defer w.listMutex.Unlock() - return w.fakeLister.List(ctx, list, opts...) -} - // NewTracker creates a new Tracker and initializes the internal trackers. func NewTracker(lister Lister, mutationEnabled, externalDataEnabled, expansionEnabled bool) *Tracker { // TODO: Dereference crashOnFailureFetchingExpectations when we change crashOnFailureFetchingExpectations to a flag From 9c5b5d70a94239e7d62c1f7136b7fccc6607fd26 Mon Sep 17 00:00:00 2001 From: Avinash Patnala Date: Fri, 20 Sep 2024 23:56:44 +0000 Subject: [PATCH 18/28] add unit tests for config pod status types Signed-off-by: Avinash Patnala --- .../v1beta1/configpodstatus_types_test.go | 68 +++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 apis/status/v1beta1/configpodstatus_types_test.go diff --git a/apis/status/v1beta1/configpodstatus_types_test.go b/apis/status/v1beta1/configpodstatus_types_test.go new file mode 100644 index 00000000000..0e5c97260ae --- /dev/null +++ b/apis/status/v1beta1/configpodstatus_types_test.go @@ -0,0 +1,68 @@ +package v1beta1_test + +import ( + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/open-policy-agent/gatekeeper/v3/apis/status/v1beta1" + "github.com/open-policy-agent/gatekeeper/v3/pkg/fakes" + "github.com/open-policy-agent/gatekeeper/v3/pkg/operations" + "github.com/open-policy-agent/gatekeeper/v3/test/testutils" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" +) + +func TestNewConfigStatusForPod(t *testing.T) { + podName := "some-gk-pod" + podNS := "a-gk-namespace" + configName := "a-config" + + testutils.Setenv(t, "POD_NAMESPACE", podNS) + + scheme := runtime.NewScheme() + err := v1beta1.AddToScheme(scheme) + if err != nil { + t.Fatal(err) + } + + err = corev1.AddToScheme(scheme) + if err != nil { + t.Fatal(err) + } + + pod := fakes.Pod( + fakes.WithNamespace(podNS), + fakes.WithName(podName), + ) + + expectedStatus := &v1beta1.ConfigPodStatus{} + expectedStatus.SetName("some--gk--pod-a--config") + expectedStatus.SetNamespace(podNS) + expectedStatus.Status.ID = podName + expectedStatus.Status.Operations = operations.AssignedStringList() + expectedStatus.SetLabels(map[string]string{ + v1beta1.ConfigNameLabel: configName, + v1beta1.PodLabel: podName, + }) + + err = controllerutil.SetOwnerReference(pod, expectedStatus, scheme) + if err != nil { + t.Fatal(err) + } + + status, err := v1beta1.NewConfigStatusForPod(pod, configName, scheme) + if err != nil { + t.Fatal(err) + } + if diff := cmp.Diff(expectedStatus, status); diff != "" { + t.Fatal(diff) + } + n, err := v1beta1.KeyForConfig(podName, configName) + if err != nil { + t.Fatal(err) + } + if status.Name != n { + t.Fatal("got status.Name != n, want equal") + } +} From 3a4e7b2a9f2d0d1346adf8f8d6ef73e874bd1197 Mon Sep 17 00:00:00 2001 From: Avinash Patnala Date: Sat, 21 Sep 2024 00:26:14 +0000 Subject: [PATCH 19/28] write upsert error to status obj Signed-off-by: Avinash Patnala --- .../v1beta1/configpodstatus_types_test.go | 6 +++--- pkg/controller/config/config_controller.go | 17 ++++++++++++++--- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/apis/status/v1beta1/configpodstatus_types_test.go b/apis/status/v1beta1/configpodstatus_types_test.go index 0e5c97260ae..0ccecd91357 100644 --- a/apis/status/v1beta1/configpodstatus_types_test.go +++ b/apis/status/v1beta1/configpodstatus_types_test.go @@ -14,9 +14,9 @@ import ( ) func TestNewConfigStatusForPod(t *testing.T) { - podName := "some-gk-pod" - podNS := "a-gk-namespace" - configName := "a-config" + const podName = "some-gk-pod" + const podNS = "a-gk-namespace" + const configName = "a-config" testutils.Setenv(t, "POD_NAMESPACE", podNS) diff --git a/pkg/controller/config/config_controller.go b/pkg/controller/config/config_controller.go index b675f502741..5254e35a3b6 100644 --- a/pkg/controller/config/config_controller.go +++ b/pkg/controller/config/config_controller.go @@ -215,7 +215,7 @@ func (r *ReconcileConfig) Reconcile(ctx context.Context, request reconcile.Reque if err := r.cacheManager.UpsertSource(ctx, configSourceKey, gvksToSync); err != nil { r.tracker.For(configGVK).TryCancelExpect(instance) - return reconcile.Result{Requeue: true}, fmt.Errorf("config-controller: error establishing watches for new syncOny: %w", err) + return reconcile.Result{Requeue: true}, r.updateOrCreatePodStatus(ctx, instance, err) } r.tracker.For(configGVK).Observe(instance) @@ -223,7 +223,7 @@ func (r *ReconcileConfig) Reconcile(ctx context.Context, request reconcile.Reque if deleted { return reconcile.Result{}, r.deleteStatus(ctx, request.NamespacedName.Name) } - return reconcile.Result{}, r.updateOrCreatePodStatus(ctx, instance) + return reconcile.Result{}, r.updateOrCreatePodStatus(ctx, instance, err) } func (r *ReconcileConfig) deleteStatus(ctx context.Context, cfgName string) error { @@ -244,7 +244,7 @@ func (r *ReconcileConfig) deleteStatus(ctx context.Context, cfgName string) erro return nil } -func (r *ReconcileConfig) updateOrCreatePodStatus(ctx context.Context, cfg *configv1alpha1.Config) error { +func (r *ReconcileConfig) updateOrCreatePodStatus(ctx context.Context, cfg *configv1alpha1.Config, upsertErr error) error { pod, err := r.getPod(ctx) if err != nil { return fmt.Errorf("getting reconciler pod: %w", err) @@ -271,6 +271,8 @@ func (r *ReconcileConfig) updateOrCreatePodStatus(ctx context.Context, cfg *conf return fmt.Errorf("getting config status in name %s, namespace %s: %w", cfg.GetName(), cfg.GetNamespace(), err) } + setStatusError(status, upsertErr) + status.Status.ObservedGeneration = cfg.GetGeneration() if shouldCreate { @@ -288,3 +290,12 @@ func (r *ReconcileConfig) newConfigStatus(pod *corev1.Pod, cfg *configv1alpha1.C return status, nil } + +func setStatusError(status *statusv1beta1.ConfigPodStatus, etErr error) { + if etErr == nil { + status.Status.Errors = nil + return + } + e := &statusv1beta1.ConfigError{Message: etErr.Error()} + status.Status.Errors = []*statusv1beta1.ConfigError{e} +} From 1d44b5f29718d214f944a741fc29deef54d27ca5 Mon Sep 17 00:00:00 2001 From: Avinash Patnala Date: Mon, 23 Sep 2024 20:50:14 +0000 Subject: [PATCH 20/28] update kubebuilder tag in config status controller Signed-off-by: Avinash Patnala --- config/rbac/role.yaml | 12 ++++++++++++ .../gatekeeper-manager-role-clusterrole.yaml | 12 ++++++++++++ manifest_staging/deploy/gatekeeper.yaml | 12 ++++++++++++ .../configstatus/configstatus_controller.go | 4 ++-- 4 files changed, 38 insertions(+), 2 deletions(-) diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index f3416ee2060..a6d56048491 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -56,6 +56,18 @@ rules: - patch - update - watch +- apiGroups: + - config.gatekeeper.sh + resources: + - '*' + verbs: + - create + - delete + - get + - list + - patch + - update + - watch - apiGroups: - config.gatekeeper.sh resources: diff --git a/manifest_staging/charts/gatekeeper/templates/gatekeeper-manager-role-clusterrole.yaml b/manifest_staging/charts/gatekeeper/templates/gatekeeper-manager-role-clusterrole.yaml index a6306b3a285..591d36dc566 100644 --- a/manifest_staging/charts/gatekeeper/templates/gatekeeper-manager-role-clusterrole.yaml +++ b/manifest_staging/charts/gatekeeper/templates/gatekeeper-manager-role-clusterrole.yaml @@ -63,6 +63,18 @@ rules: - patch - update - watch +- apiGroups: + - config.gatekeeper.sh + resources: + - '*' + verbs: + - create + - delete + - get + - list + - patch + - update + - watch - apiGroups: - config.gatekeeper.sh resources: diff --git a/manifest_staging/deploy/gatekeeper.yaml b/manifest_staging/deploy/gatekeeper.yaml index 8d83849cf93..a45835556c0 100644 --- a/manifest_staging/deploy/gatekeeper.yaml +++ b/manifest_staging/deploy/gatekeeper.yaml @@ -4826,6 +4826,18 @@ rules: - patch - update - watch +- apiGroups: + - config.gatekeeper.sh + resources: + - '*' + verbs: + - create + - delete + - get + - list + - patch + - update + - watch - apiGroups: - config.gatekeeper.sh resources: diff --git a/pkg/controller/configstatus/configstatus_controller.go b/pkg/controller/configstatus/configstatus_controller.go index 99bd124d897..a59f731901c 100644 --- a/pkg/controller/configstatus/configstatus_controller.go +++ b/pkg/controller/configstatus/configstatus_controller.go @@ -131,10 +131,10 @@ type ReconcileConfigStatus struct { log logr.Logger } -// +kubebuilder:rbac:groups=expansion.gatekeeper.sh,resources=*,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups=config.gatekeeper.sh,resources=*,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=status.gatekeeper.sh,resources=*,verbs=get;list;watch;create;update;patch;delete -// Reconcile reads that state of the cluster for a constraint object and makes changes based on the state read +// Reconcile reads that state of the cluster for a config object and makes changes based on the state read // and what is in the constraint.Spec. func (r *ReconcileConfigStatus) Reconcile(ctx context.Context, request reconcile.Request) (reconcile.Result, error) { et := &configv1alpha1.Config{} From f371fb3a0533515f63a02eadd721751a53a20b75 Mon Sep 17 00:00:00 2001 From: Avinash Patnala Date: Mon, 23 Sep 2024 21:42:28 +0000 Subject: [PATCH 21/28] pass namespace to reconcile request of config controller Signed-off-by: Avinash Patnala --- pkg/controller/configstatus/configstatus_controller.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pkg/controller/configstatus/configstatus_controller.go b/pkg/controller/configstatus/configstatus_controller.go index a59f731901c..5ba50e1570d 100644 --- a/pkg/controller/configstatus/configstatus_controller.go +++ b/pkg/controller/configstatus/configstatus_controller.go @@ -93,7 +93,11 @@ func PodStatusToConfigMapper(selfOnly bool) handler.TypedMapFunc[*v1beta1.Config return nil } } - return []reconcile.Request{{NamespacedName: types.NamespacedName{Name: name}}} + + return []reconcile.Request{{NamespacedName: types.NamespacedName{ + Name: name, + Namespace: util.GetNamespace(), + }}} } } From 45c2da7da26c9888ad78138ace5e7d97982dceb2 Mon Sep 17 00:00:00 2001 From: Avinash Patnala Date: Mon, 23 Sep 2024 22:29:30 +0000 Subject: [PATCH 22/28] address review comments Signed-off-by: Avinash Patnala --- pkg/controller/configstatus/configstatus_controller.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/controller/configstatus/configstatus_controller.go b/pkg/controller/configstatus/configstatus_controller.go index 5ba50e1570d..e700a05bef6 100644 --- a/pkg/controller/configstatus/configstatus_controller.go +++ b/pkg/controller/configstatus/configstatus_controller.go @@ -141,8 +141,8 @@ type ReconcileConfigStatus struct { // Reconcile reads that state of the cluster for a config object and makes changes based on the state read // and what is in the constraint.Spec. func (r *ReconcileConfigStatus) Reconcile(ctx context.Context, request reconcile.Request) (reconcile.Result, error) { - et := &configv1alpha1.Config{} - err := r.reader.Get(ctx, request.NamespacedName, et) + cfg := &configv1alpha1.Config{} + err := r.reader.Get(ctx, request.NamespacedName, cfg) if err != nil { // If the Config does not exist then we are done if errors.IsNotFound(err) { @@ -170,15 +170,15 @@ func (r *ReconcileConfigStatus) Reconcile(ctx context.Context, request reconcile // Don't report status if it's not for the correct object. This can happen // if a watch gets interrupted, causing the constraint status to be deleted // out from underneath it - if statusObjs[i].Status.ConfigUID != et.GetUID() { + if statusObjs[i].Status.ConfigUID != cfg.GetUID() { continue } s = append(s, statusObjs[i].Status) } - et.Status.ByPod = s + cfg.Status.ByPod = s - if err := r.statusClient.Status().Update(ctx, et); err != nil { + if err := r.statusClient.Status().Update(ctx, cfg); err != nil { return reconcile.Result{Requeue: true}, nil } return reconcile.Result{}, nil From 4dae25f7fdd9d4676a5b85637e4359dd44b288ff Mon Sep 17 00:00:00 2001 From: Avinash Patnala Date: Tue, 24 Sep 2024 00:16:26 +0000 Subject: [PATCH 23/28] use namespace stored in config pod status Signed-off-by: Avinash Patnala --- pkg/controller/configstatus/configstatus_controller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/controller/configstatus/configstatus_controller.go b/pkg/controller/configstatus/configstatus_controller.go index e700a05bef6..8e647d95d54 100644 --- a/pkg/controller/configstatus/configstatus_controller.go +++ b/pkg/controller/configstatus/configstatus_controller.go @@ -96,7 +96,7 @@ func PodStatusToConfigMapper(selfOnly bool) handler.TypedMapFunc[*v1beta1.Config return []reconcile.Request{{NamespacedName: types.NamespacedName{ Name: name, - Namespace: util.GetNamespace(), + Namespace: obj.Namespace, }}} } } From b32605920ee817dcad709fd1e4ad380971594c24 Mon Sep 17 00:00:00 2001 From: Avinash Patnala Date: Tue, 24 Sep 2024 23:23:59 +0000 Subject: [PATCH 24/28] store namespace in configpod status name Signed-off-by: Avinash Patnala --- apis/status/v1beta1/configpodstatus_types.go | 8 ++++---- apis/status/v1beta1/configpodstatus_types_test.go | 7 ++++--- pkg/controller/config/config_controller.go | 10 +++++----- 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/apis/status/v1beta1/configpodstatus_types.go b/apis/status/v1beta1/configpodstatus_types.go index 46cdc955dae..641bbfdd35c 100644 --- a/apis/status/v1beta1/configpodstatus_types.go +++ b/apis/status/v1beta1/configpodstatus_types.go @@ -57,9 +57,9 @@ func init() { // NewConfigStatusForPod returns an config status object // that has been initialized with the bare minimum of fields to make it functional // with the config status controller. -func NewConfigStatusForPod(pod *corev1.Pod, configName string, scheme *runtime.Scheme) (*ConfigPodStatus, error) { +func NewConfigStatusForPod(pod *corev1.Pod, configNamespace string, configName string, scheme *runtime.Scheme) (*ConfigPodStatus, error) { obj := &ConfigPodStatus{} - name, err := KeyForConfig(pod.Name, configName) + name, err := KeyForConfig(pod.Name, configNamespace, configName) if err != nil { return nil, err } @@ -81,6 +81,6 @@ func NewConfigStatusForPod(pod *corev1.Pod, configName string, scheme *runtime.S // KeyForConfig returns a unique status object name given the Pod ID and // a config object. -func KeyForConfig(id string, configName string) (string, error) { - return DashPacker(id, configName) +func KeyForConfig(id string, configNamespace string, configName string) (string, error) { + return DashPacker(id, configNamespace, configName) } diff --git a/apis/status/v1beta1/configpodstatus_types_test.go b/apis/status/v1beta1/configpodstatus_types_test.go index 0ccecd91357..7e9c73a0e9a 100644 --- a/apis/status/v1beta1/configpodstatus_types_test.go +++ b/apis/status/v1beta1/configpodstatus_types_test.go @@ -17,6 +17,7 @@ func TestNewConfigStatusForPod(t *testing.T) { const podName = "some-gk-pod" const podNS = "a-gk-namespace" const configName = "a-config" + const configNameSpace = "a-gk-ns" testutils.Setenv(t, "POD_NAMESPACE", podNS) @@ -37,7 +38,7 @@ func TestNewConfigStatusForPod(t *testing.T) { ) expectedStatus := &v1beta1.ConfigPodStatus{} - expectedStatus.SetName("some--gk--pod-a--config") + expectedStatus.SetName("some--gk--pod-a--gk--ns-a--config") expectedStatus.SetNamespace(podNS) expectedStatus.Status.ID = podName expectedStatus.Status.Operations = operations.AssignedStringList() @@ -51,14 +52,14 @@ func TestNewConfigStatusForPod(t *testing.T) { t.Fatal(err) } - status, err := v1beta1.NewConfigStatusForPod(pod, configName, scheme) + status, err := v1beta1.NewConfigStatusForPod(pod, configNameSpace, configName, scheme) if err != nil { t.Fatal(err) } if diff := cmp.Diff(expectedStatus, status); diff != "" { t.Fatal(diff) } - n, err := v1beta1.KeyForConfig(podName, configName) + n, err := v1beta1.KeyForConfig(podName, configNameSpace, configName) if err != nil { t.Fatal(err) } diff --git a/pkg/controller/config/config_controller.go b/pkg/controller/config/config_controller.go index 5254e35a3b6..d085493450c 100644 --- a/pkg/controller/config/config_controller.go +++ b/pkg/controller/config/config_controller.go @@ -221,18 +221,18 @@ func (r *ReconcileConfig) Reconcile(ctx context.Context, request reconcile.Reque r.tracker.For(configGVK).Observe(instance) if deleted { - return reconcile.Result{}, r.deleteStatus(ctx, request.NamespacedName.Name) + return reconcile.Result{}, r.deleteStatus(ctx, request.NamespacedName.Namespace, request.NamespacedName.Name) } return reconcile.Result{}, r.updateOrCreatePodStatus(ctx, instance, err) } -func (r *ReconcileConfig) deleteStatus(ctx context.Context, cfgName string) error { +func (r *ReconcileConfig) deleteStatus(ctx context.Context, cfgNamespace string, cfgName string) error { status := &statusv1beta1.ConfigPodStatus{} pod, err := r.getPod(ctx) if err != nil { return fmt.Errorf("getting reconciler pod: %w", err) } - sName, err := statusv1beta1.KeyForConfig(pod.Name, cfgName) + sName, err := statusv1beta1.KeyForConfig(pod.Name, cfgNamespace, cfgName) if err != nil { return fmt.Errorf("getting key for config: %w", err) } @@ -252,7 +252,7 @@ func (r *ReconcileConfig) updateOrCreatePodStatus(ctx context.Context, cfg *conf // Check if it exists already sNS := pod.Namespace - sName, err := statusv1beta1.KeyForConfig(pod.Name, cfg.GetName()) + sName, err := statusv1beta1.KeyForConfig(pod.Name, cfg.GetNamespace(), cfg.GetName()) if err != nil { return fmt.Errorf("getting key for config: %w", err) } @@ -282,7 +282,7 @@ func (r *ReconcileConfig) updateOrCreatePodStatus(ctx context.Context, cfg *conf } func (r *ReconcileConfig) newConfigStatus(pod *corev1.Pod, cfg *configv1alpha1.Config) (*statusv1beta1.ConfigPodStatus, error) { - status, err := statusv1beta1.NewConfigStatusForPod(pod, cfg.GetName(), r.scheme) + status, err := statusv1beta1.NewConfigStatusForPod(pod, cfg.GetNamespace(), cfg.GetName(), r.scheme) if err != nil { return nil, fmt.Errorf("creating status for pod: %w", err) } From b75a3eaef3696416e7407b8819d8832e31c6b696 Mon Sep 17 00:00:00 2001 From: Avinash Patnala Date: Wed, 25 Sep 2024 18:25:35 +0000 Subject: [PATCH 25/28] address review comments Signed-off-by: Avinash Patnala --- apis/status/v1beta1/configpodstatus_types.go | 8 ++++++++ pkg/controller/config/config_controller.go | 2 +- pkg/controller/configstatus/configstatus_controller.go | 4 ++-- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/apis/status/v1beta1/configpodstatus_types.go b/apis/status/v1beta1/configpodstatus_types.go index 641bbfdd35c..014fa69739e 100644 --- a/apis/status/v1beta1/configpodstatus_types.go +++ b/apis/status/v1beta1/configpodstatus_types.go @@ -81,6 +81,14 @@ func NewConfigStatusForPod(pod *corev1.Pod, configNamespace string, configName s // KeyForConfig returns a unique status object name given the Pod ID and // a config object. +// The object name must satisfy RFC 1123 Label Names spec +// (https://kubernetes.io/docs/concepts/overview/working-with-objects/names/) +// and Kubernetes validation rules for object names. +// +// It's possible that dash packing/unpacking would result in a name +// that exceeds the maximum length allowed, but for Config resources, +// the configName should always be "config", and namespace would be "gatekeeper-system", +// so this validation will hold. func KeyForConfig(id string, configNamespace string, configName string) (string, error) { return DashPacker(id, configNamespace, configName) } diff --git a/pkg/controller/config/config_controller.go b/pkg/controller/config/config_controller.go index d085493450c..afe0cd7adc7 100644 --- a/pkg/controller/config/config_controller.go +++ b/pkg/controller/config/config_controller.go @@ -223,7 +223,7 @@ func (r *ReconcileConfig) Reconcile(ctx context.Context, request reconcile.Reque if deleted { return reconcile.Result{}, r.deleteStatus(ctx, request.NamespacedName.Namespace, request.NamespacedName.Name) } - return reconcile.Result{}, r.updateOrCreatePodStatus(ctx, instance, err) + return reconcile.Result{}, r.updateOrCreatePodStatus(ctx, instance, nil) } func (r *ReconcileConfig) deleteStatus(ctx context.Context, cfgNamespace string, cfgName string) error { diff --git a/pkg/controller/configstatus/configstatus_controller.go b/pkg/controller/configstatus/configstatus_controller.go index 8e647d95d54..b0bae57e02a 100644 --- a/pkg/controller/configstatus/configstatus_controller.go +++ b/pkg/controller/configstatus/configstatus_controller.go @@ -104,7 +104,6 @@ func PodStatusToConfigMapper(selfOnly bool) handler.TypedMapFunc[*v1beta1.Config // Add creates a new config status Controller and adds it to the Manager. The Manager will set fields on the Controller // and Start it when the Manager is Started. func add(mgr manager.Manager, r reconcile.Reconciler) error { - // Create a new controller c, err := controller.New("config-status-controller", mgr, controller.Options{Reconciler: r}) if err != nil { return err @@ -125,7 +124,8 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error { var _ reconcile.Reconciler = &ReconcileConfigStatus{} -// ReconcileConfigStatus reconciles an arbitrary config object described by Kind. +// ReconcileConfigStatus provides the dependencies required to reconcile +// the status of a Config resource. type ReconcileConfigStatus struct { reader client.Reader writer client.Writer From 8201cff75cec2fb323bef547e5d5d72caa627b1d Mon Sep 17 00:00:00 2001 From: Avinash Patnala Date: Tue, 1 Oct 2024 18:13:40 +0000 Subject: [PATCH 26/28] wrap fake client with mutex in unit tests Signed-off-by: Avinash Patnala --- pkg/readiness/ready_tracker_unit_test.go | 59 +++++++++++++----------- 1 file changed, 33 insertions(+), 26 deletions(-) diff --git a/pkg/readiness/ready_tracker_unit_test.go b/pkg/readiness/ready_tracker_unit_test.go index d2adf8c9ab0..94a1f9b9da6 100644 --- a/pkg/readiness/ready_tracker_unit_test.go +++ b/pkg/readiness/ready_tracker_unit_test.go @@ -279,6 +279,9 @@ func Test_Tracker_TryCancelData(t *testing.T) { lister := fake.NewClientBuilder().WithScheme(mustInitializeScheme(runtime.NewScheme())).WithRuntimeObjects( &testSyncSet, fakes.UnstructuredFor(podGVK, "", "pod1-name"), ).Build() + + wrapLister := WrapFakeClientWithMutex{fakeLister: lister} + tcs := []struct { name string retries int @@ -292,7 +295,7 @@ func Test_Tracker_TryCancelData(t *testing.T) { objDataFn := func() objData { return objData{retries: tc.retries} } - rt := newTracker(lister, false, false, false, false, nil, objDataFn) + rt := newTracker(&wrapLister, false, false, false, false, nil, objDataFn) ctx, cancel := context.WithCancel(context.Background()) var runErr error @@ -361,9 +364,10 @@ func Test_ReadyTracker_TrackAssignMetadata(t *testing.T) { } return client.List(ctx, list, opts...) } - lister := fake.NewClientBuilder().WithScheme(mustInitializeScheme(runtime.NewScheme())).WithRuntimeObjects(&testAssignMetadata).WithInterceptorFuncs(*funcs).Build() - rt := newTracker(lister, true, false, false, tc.failClose, retryNone, func() objData { + wrapLister := WrapFakeClientWithMutex{fakeLister: lister} + + rt := newTracker(&wrapLister, true, false, false, tc.failClose, retryNone, func() objData { return objData{retries: 0} }) errChan := make(chan error) @@ -432,7 +436,8 @@ func Test_ReadyTracker_TrackAssign(t *testing.T) { } lister := fake.NewClientBuilder().WithScheme(mustInitializeScheme(runtime.NewScheme())).WithRuntimeObjects(&testAssign).WithInterceptorFuncs(*funcs).Build() - rt := newTracker(lister, true, false, false, tc.failClose, retryNone, func() objData { + wrapLister := WrapFakeClientWithMutex{fakeLister: lister} + rt := newTracker(&wrapLister, true, false, false, tc.failClose, retryNone, func() objData { return objData{retries: 0} }) errChan := make(chan error) @@ -501,7 +506,8 @@ func Test_ReadyTracker_TrackModifySet(t *testing.T) { } lister := fake.NewClientBuilder().WithScheme(mustInitializeScheme(runtime.NewScheme())).WithRuntimeObjects(&testModifySet).WithInterceptorFuncs(*funcs).Build() - rt := newTracker(lister, true, false, false, tc.failClose, retryNone, func() objData { + wrapLister := WrapFakeClientWithMutex{fakeLister: lister} + rt := newTracker(&wrapLister, true, false, false, tc.failClose, retryNone, func() objData { return objData{retries: 0} }) errChan := make(chan error) @@ -571,7 +577,8 @@ func Test_ReadyTracker_TrackAssignImage(t *testing.T) { } lister := fake.NewClientBuilder().WithScheme(mustInitializeScheme(runtime.NewScheme())).WithRuntimeObjects(&testAssignImage).WithInterceptorFuncs(*funcs).Build() - rt := newTracker(lister, true, false, false, tc.failClose, retryNone, func() objData { + wrapLister := WrapFakeClientWithMutex{fakeLister: lister} + rt := newTracker(&wrapLister, true, false, false, tc.failClose, retryNone, func() objData { return objData{retries: 0} }) errChan := make(chan error) @@ -640,7 +647,8 @@ func Test_ReadyTracker_TrackExternalDataProvider(t *testing.T) { } lister := fake.NewClientBuilder().WithScheme(mustInitializeScheme(runtime.NewScheme())).WithRuntimeObjects(&testExternalDataProvider).WithInterceptorFuncs(*funcs).Build() - rt := newTracker(lister, false, true, false, tc.failClose, retryNone, func() objData { + wrapLister := WrapFakeClientWithMutex{fakeLister: lister} + rt := newTracker(&wrapLister, false, true, false, tc.failClose, retryNone, func() objData { return objData{retries: 0} }) errChan := make(chan error) @@ -709,7 +717,8 @@ func Test_ReadyTracker_TrackExpansionTemplates(t *testing.T) { } lister := fake.NewClientBuilder().WithScheme(mustInitializeScheme(runtime.NewScheme())).WithRuntimeObjects(&testExpansionTemplate).WithInterceptorFuncs(*funcs).Build() - rt := newTracker(lister, false, false, true, tc.failClose, retryNone, func() objData { + wrapLister := WrapFakeClientWithMutex{fakeLister: lister} + rt := newTracker(&wrapLister, false, false, true, tc.failClose, retryNone, func() objData { return objData{retries: 0} }) errChan := make(chan error) @@ -778,7 +787,8 @@ func Test_ReadyTracker_TrackConstraintTemplates(t *testing.T) { } lister := fake.NewClientBuilder().WithScheme(mustInitializeScheme(runtime.NewScheme())).WithRuntimeObjects(convertedTemplate.DeepCopyObject()).WithInterceptorFuncs(*funcs).Build() - rt := newTracker(lister, false, false, false, tc.failClose, retryNone, func() objData { + wrapLister := WrapFakeClientWithMutex{fakeLister: lister} + rt := newTracker(&wrapLister, false, false, false, tc.failClose, retryNone, func() objData { return objData{retries: 0} }) errChan := make(chan error) @@ -881,7 +891,8 @@ func Test_ReadyTracker_TrackConfigAndSyncSets(t *testing.T) { return client.List(ctx, list, opts...) } lister := fake.NewClientBuilder().WithScheme(mustInitializeScheme(runtime.NewScheme())).WithRuntimeObjects(&testSyncSet, &testConfig).WithInterceptorFuncs(*funcs).Build() - rt := newTracker(lister, false, false, false, tc.failClose, retryNone, func() objData { + wrapLister := WrapFakeClientWithMutex{fakeLister: lister} + rt := newTracker(&wrapLister, false, false, false, tc.failClose, retryNone, func() objData { return objData{retries: 0} }) errChan := make(chan error) @@ -969,7 +980,8 @@ func Test_ReadyTracker_TrackConstraint(t *testing.T) { } lister := fake.NewClientBuilder().WithScheme(mustInitializeScheme(runtime.NewScheme())).WithRuntimeObjects(getTestConstraint()).WithInterceptorFuncs(*funcs).Build() - rt := newTracker(lister, false, false, false, tc.failClose, retryNone, func() objData { + wrapLister := WrapFakeClientWithMutex{fakeLister: lister} + rt := newTracker(&wrapLister, false, false, false, tc.failClose, retryNone, func() objData { return objData{retries: 0} }) errChan := make(chan error) @@ -1045,7 +1057,8 @@ func Test_ReadyTracker_TrackData(t *testing.T) { } lister := fake.NewClientBuilder().WithScheme(mustInitializeScheme(runtime.NewScheme())).WithRuntimeObjects(fakes.UnstructuredFor(podGVK, "", "pod1-name")).WithInterceptorFuncs(*funcs).Build() - rt := newTracker(lister, false, false, false, tc.failClose, retryNone, func() objData { + wrapLister := WrapFakeClientWithMutex{fakeLister: lister} + rt := newTracker(&wrapLister, false, false, false, tc.failClose, retryNone, func() objData { return objData{retries: 0} }) errChan := make(chan error) @@ -1106,21 +1119,17 @@ func Test_ReadyTracker_Run_GRP_Wait(t *testing.T) { } for _, tc := range tcs { t.Run(tc.name, func(t *testing.T) { - var m sync.Mutex funcs := &interceptor.Funcs{} funcs.List = func(ctx context.Context, client client.WithWatch, list client.ObjectList, opts ...client.ListOption) error { if _, ok := list.(*v1beta1.ConstraintTemplateList); ok { return fmt.Errorf("Force Test ConstraintTemplateList Failure") } - - // Adding a mutex lock here avoids the race condition within fake client's List method - m.Lock() - defer m.Unlock() return client.List(ctx, list, opts...) } lister := fake.NewClientBuilder().WithScheme(mustInitializeScheme(runtime.NewScheme())).WithRuntimeObjects(&testExpansionTemplate, convertedTemplate.DeepCopyObject(), getTestConstraint(), &testSyncSet, fakes.UnstructuredFor(podGVK, "", "pod1-name")).WithInterceptorFuncs(*funcs).Build() - rt := newTracker(lister, false, false, true, tc.failClose, retryNone, func() objData { + wrapLister := WrapFakeClientWithMutex{fakeLister: lister} + rt := newTracker(&wrapLister, false, false, true, tc.failClose, retryNone, func() objData { return objData{retries: 0} }) @@ -1160,20 +1169,19 @@ func Test_ReadyTracker_Run_ConstraintTrackers_Wait(t *testing.T) { } for _, tc := range tcs { t.Run(tc.name, func(t *testing.T) { - var m sync.Mutex funcs := &interceptor.Funcs{} funcs.List = func(ctx context.Context, client client.WithWatch, list client.ObjectList, opts ...client.ListOption) error { if v, ok := list.(*unstructured.UnstructuredList); ok && v.GroupVersionKind().Group == "constraints.gatekeeper.sh" { t.Log(v.GroupVersionKind()) return fmt.Errorf("Force Test constraint list Failure") } - m.Lock() - defer m.Unlock() + return client.List(ctx, list, opts...) } lister := fake.NewClientBuilder().WithScheme(mustInitializeScheme(runtime.NewScheme())).WithRuntimeObjects(&testExpansionTemplate, convertedTemplate.DeepCopyObject(), getTestConstraint(), &testSyncSet, fakes.UnstructuredFor(podGVK, "", "pod1-name")).WithInterceptorFuncs(*funcs).Build() - rt := newTracker(lister, false, false, true, tc.failClose, retryNone, func() objData { + wrapLister := WrapFakeClientWithMutex{fakeLister: lister} + rt := newTracker(&wrapLister, false, false, true, tc.failClose, retryNone, func() objData { return objData{retries: 0} }) @@ -1214,19 +1222,18 @@ func Test_ReadyTracker_Run_DataTrackers_Wait(t *testing.T) { } for _, tc := range tcs { t.Run(tc.name, func(t *testing.T) { - var m sync.Mutex funcs := &interceptor.Funcs{} funcs.List = func(ctx context.Context, client client.WithWatch, list client.ObjectList, opts ...client.ListOption) error { if v, ok := list.(*unstructured.UnstructuredList); ok && v.GroupVersionKind().Kind == "PodList" { return fmt.Errorf("Force Test pod list Failure") } - m.Lock() - defer m.Unlock() + return client.List(ctx, list, opts...) } lister := fake.NewClientBuilder().WithScheme(mustInitializeScheme(runtime.NewScheme())).WithRuntimeObjects(&testExpansionTemplate, convertedTemplate.DeepCopyObject(), getTestConstraint(), &testSyncSet, fakes.UnstructuredFor(podGVK, "", "pod1-name")).WithInterceptorFuncs(*funcs).Build() - rt := newTracker(lister, false, false, true, tc.failClose, retryNone, func() objData { + wrapLister := WrapFakeClientWithMutex{fakeLister: lister} + rt := newTracker(&wrapLister, false, false, true, tc.failClose, retryNone, func() objData { return objData{retries: 0} }) From 958be2ccd677e9fac2555201672e1c1943ee8334 Mon Sep 17 00:00:00 2001 From: Avinash Patnala Date: Tue, 1 Oct 2024 20:45:16 +0000 Subject: [PATCH 27/28] fix expansion template status to use self only and gate all status controllers to use operation.status flag Signed-off-by: Avinash Patnala --- .../constraintstatus_controller.go | 4 ++++ .../constrainttemplatestatus_controller.go | 4 ++++ pkg/controller/expansion/expansion_controller.go | 14 +++++++++++++- .../expansionstatus/expansionstatus_controller.go | 4 ++++ .../mutatorstatus/mutatorstatus_controller.go | 3 +++ 5 files changed, 28 insertions(+), 1 deletion(-) diff --git a/pkg/controller/constraintstatus/constraintstatus_controller.go b/pkg/controller/constraintstatus/constraintstatus_controller.go index 16f293894d4..7f6714c85f2 100644 --- a/pkg/controller/constraintstatus/constraintstatus_controller.go +++ b/pkg/controller/constraintstatus/constraintstatus_controller.go @@ -25,6 +25,7 @@ import ( constraintclient "github.com/open-policy-agent/frameworks/constraint/pkg/client" "github.com/open-policy-agent/gatekeeper/v3/apis/status/v1beta1" "github.com/open-policy-agent/gatekeeper/v3/pkg/logging" + "github.com/open-policy-agent/gatekeeper/v3/pkg/operations" "github.com/open-policy-agent/gatekeeper/v3/pkg/util" "github.com/open-policy-agent/gatekeeper/v3/pkg/watch" "k8s.io/apimachinery/pkg/api/errors" @@ -54,6 +55,9 @@ type Adder struct { // Add creates a new Constraint Status Controller and adds it to the Manager. The Manager will set fields on the Controller // and Start it when the Manager is Started. func (a *Adder) Add(mgr manager.Manager) error { + if !operations.IsAssigned(operations.Status) { + return nil + } r := newReconciler(mgr, a.ControllerSwitch) if a.IfWatching != nil { r.ifWatching = a.IfWatching diff --git a/pkg/controller/constrainttemplatestatus/constrainttemplatestatus_controller.go b/pkg/controller/constrainttemplatestatus/constrainttemplatestatus_controller.go index 1b1f15f4608..6947a2c3ad8 100644 --- a/pkg/controller/constrainttemplatestatus/constrainttemplatestatus_controller.go +++ b/pkg/controller/constrainttemplatestatus/constrainttemplatestatus_controller.go @@ -26,6 +26,7 @@ import ( constraintclient "github.com/open-policy-agent/frameworks/constraint/pkg/client" "github.com/open-policy-agent/gatekeeper/v3/apis/status/v1beta1" "github.com/open-policy-agent/gatekeeper/v3/pkg/logging" + "github.com/open-policy-agent/gatekeeper/v3/pkg/operations" "github.com/open-policy-agent/gatekeeper/v3/pkg/util" "github.com/open-policy-agent/gatekeeper/v3/pkg/watch" "k8s.io/apimachinery/pkg/api/errors" @@ -52,6 +53,9 @@ type Adder struct { // Add creates a new Constraint Status Controller and adds it to the Manager. The Manager will set fields on the Controller // and Start it when the Manager is Started. func (a *Adder) Add(mgr manager.Manager) error { + if !operations.IsAssigned(operations.Status) { + return nil + } r := newReconciler(mgr, a.ControllerSwitch) return add(mgr, r) } diff --git a/pkg/controller/expansion/expansion_controller.go b/pkg/controller/expansion/expansion_controller.go index 0aa3ceb89e2..3539508adf0 100644 --- a/pkg/controller/expansion/expansion_controller.go +++ b/pkg/controller/expansion/expansion_controller.go @@ -7,6 +7,7 @@ import ( "github.com/open-policy-agent/gatekeeper/v3/apis/expansion/unversioned" expansionv1beta1 "github.com/open-policy-agent/gatekeeper/v3/apis/expansion/v1beta1" statusv1beta1 "github.com/open-policy-agent/gatekeeper/v3/apis/status/v1beta1" + "github.com/open-policy-agent/gatekeeper/v3/pkg/controller/expansionstatus" "github.com/open-policy-agent/gatekeeper/v3/pkg/expansion" "github.com/open-policy-agent/gatekeeper/v3/pkg/logging" "github.com/open-policy-agent/gatekeeper/v3/pkg/metrics" @@ -110,9 +111,20 @@ func add(mgr manager.Manager, r *Reconciler) error { } // Watch for changes to ExpansionTemplates - return c.Watch( + err = c.Watch( source.Kind(mgr.GetCache(), &expansionv1beta1.ExpansionTemplate{}, &handler.TypedEnqueueRequestForObject[*expansionv1beta1.ExpansionTemplate]{})) + if err != nil { + return err + } + + // Watch for changes to ExpansionTemplateStatuses + err = c.Watch( + source.Kind(mgr.GetCache(), &statusv1beta1.ExpansionTemplatePodStatus{}, handler.TypedEnqueueRequestsFromMapFunc(expansionstatus.PodStatusToExpansionTemplateMapper(true)))) + if err != nil { + return err + } + return nil } func (r *Reconciler) Reconcile(ctx context.Context, request reconcile.Request) (reconcile.Result, error) { diff --git a/pkg/controller/expansionstatus/expansionstatus_controller.go b/pkg/controller/expansionstatus/expansionstatus_controller.go index 3f07fee22de..e512860f856 100644 --- a/pkg/controller/expansionstatus/expansionstatus_controller.go +++ b/pkg/controller/expansionstatus/expansionstatus_controller.go @@ -26,6 +26,7 @@ import ( "github.com/open-policy-agent/gatekeeper/v3/apis/status/v1beta1" "github.com/open-policy-agent/gatekeeper/v3/pkg/expansion" "github.com/open-policy-agent/gatekeeper/v3/pkg/logging" + "github.com/open-policy-agent/gatekeeper/v3/pkg/operations" "github.com/open-policy-agent/gatekeeper/v3/pkg/readiness" "github.com/open-policy-agent/gatekeeper/v3/pkg/util" "github.com/open-policy-agent/gatekeeper/v3/pkg/watch" @@ -58,6 +59,9 @@ func (a *Adder) Add(mgr manager.Manager) error { if !*expansion.ExpansionEnabled { return nil } + if !operations.IsAssigned(operations.Status) { + return nil + } r := newReconciler(mgr) return add(mgr, r) } diff --git a/pkg/controller/mutatorstatus/mutatorstatus_controller.go b/pkg/controller/mutatorstatus/mutatorstatus_controller.go index bc63c784fe7..6167647846b 100644 --- a/pkg/controller/mutatorstatus/mutatorstatus_controller.go +++ b/pkg/controller/mutatorstatus/mutatorstatus_controller.go @@ -60,6 +60,9 @@ func (a *Adder) Add(mgr manager.Manager) error { if !operations.IsAssigned(operations.MutationStatus) { return nil } + if !operations.IsAssigned(operations.Status) { + return nil + } r := newReconciler(mgr, a.ControllerSwitch) return add(mgr, r) } From a860f6cda8319d7187c29b2d6acbe2b0d7de57bb Mon Sep 17 00:00:00 2001 From: Avinash Patnala Date: Wed, 2 Oct 2024 17:33:20 +0000 Subject: [PATCH 28/28] remove gating in mutator status controller Signed-off-by: Avinash Patnala --- pkg/controller/mutatorstatus/mutatorstatus_controller.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/pkg/controller/mutatorstatus/mutatorstatus_controller.go b/pkg/controller/mutatorstatus/mutatorstatus_controller.go index 6167647846b..bc63c784fe7 100644 --- a/pkg/controller/mutatorstatus/mutatorstatus_controller.go +++ b/pkg/controller/mutatorstatus/mutatorstatus_controller.go @@ -60,9 +60,6 @@ func (a *Adder) Add(mgr manager.Manager) error { if !operations.IsAssigned(operations.MutationStatus) { return nil } - if !operations.IsAssigned(operations.Status) { - return nil - } r := newReconciler(mgr, a.ControllerSwitch) return add(mgr, r) }