From fdceaed1d0fd403dc350e182086fb71926634e3b Mon Sep 17 00:00:00 2001 From: Jakub Warczarek Date: Fri, 20 Sep 2024 09:40:13 +0200 Subject: [PATCH] feat: require ReferenceGrant for Secrets referenced by KongPluginInstallation (#615) --- CHANGELOG.md | 2 +- api/v1alpha1/kongplugin_installation_types.go | 4 +- api/v1alpha1/zz_generated.deepcopy.go | 6 +- ...or.konghq.com_kongplugininstallations.yaml | 38 +++++++-- .../kongplugininstallation/controller.go | 83 ++++++++++++++++--- docs/api-reference.md | 2 +- .../test_kongplugininstallation.go | 58 +++++++++++-- 7 files changed, 163 insertions(+), 30 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5251f176b..ad8d82a8a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,7 +31,7 @@ [#387](https://github.com/Kong/gateway-operator/pull/387) - Introduce `KongPluginInstallation` CRD to allow installing custom Kong plugins distributed as container images. - [#400](https://github.com/Kong/gateway-operator/pull/400), [#424](https://github.com/Kong/gateway-operator/pull/424), [#474](https://github.com/Kong/gateway-operator/pull/474), [#560](https://github.com/Kong/gateway-operator/pull/560) + [#400](https://github.com/Kong/gateway-operator/pull/400), [#424](https://github.com/Kong/gateway-operator/pull/424), [#474](https://github.com/Kong/gateway-operator/pull/474), [#560](https://github.com/Kong/gateway-operator/pull/560), [#615](https://github.com/Kong/gateway-operator/pull/615) - Extended `DataPlane` API with a possibility to specify `PodDisruptionBudget` to be created for the `DataPlane` deployments via `spec.resources.podDisruptionBudget`. [#464](https://github.com/Kong/gateway-operator/pull/464) diff --git a/api/v1alpha1/kongplugin_installation_types.go b/api/v1alpha1/kongplugin_installation_types.go index bfde2bf03..4760a7ab2 100644 --- a/api/v1alpha1/kongplugin_installation_types.go +++ b/api/v1alpha1/kongplugin_installation_types.go @@ -17,8 +17,8 @@ limitations under the License. package v1alpha1 import ( - corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" ) func init() { @@ -66,7 +66,7 @@ type KongPluginInstallationSpec struct { // It is optional. If the image is public, omit this field. // //+optional - ImagePullSecretRef *corev1.SecretReference `json:"imagePullSecretRef,omitempty"` + ImagePullSecretRef *gatewayv1.SecretObjectReference `json:"imagePullSecretRef,omitempty"` } // KongPluginInstallationStatus defines the observed state of KongPluginInstallation. diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index caa2f69ef..a07ff1c0f 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -21,9 +21,9 @@ limitations under the License. package v1alpha1 import ( - corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/apis/meta/v1" runtime "k8s.io/apimachinery/pkg/runtime" + apisv1 "sigs.k8s.io/gateway-api/apis/v1" ) // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. @@ -556,8 +556,8 @@ func (in *KongPluginInstallationSpec) DeepCopyInto(out *KongPluginInstallationSp *out = *in if in.ImagePullSecretRef != nil { in, out := &in.ImagePullSecretRef, &out.ImagePullSecretRef - *out = new(corev1.SecretReference) - **out = **in + *out = new(apisv1.SecretObjectReference) + (*in).DeepCopyInto(*out) } } diff --git a/config/crd/bases/gateway-operator.konghq.com_kongplugininstallations.yaml b/config/crd/bases/gateway-operator.konghq.com_kongplugininstallations.yaml index bbb1eed87..cd9254a7c 100644 --- a/config/crd/bases/gateway-operator.konghq.com_kongplugininstallations.yaml +++ b/config/crd/bases/gateway-operator.konghq.com_kongplugininstallations.yaml @@ -63,16 +63,44 @@ spec: in Image. It must follow the format in https://kubernetes.io/docs/tasks/configure-pod-container/pull-image-private-registry. It is optional. If the image is public, omit this field. properties: + group: + default: "" + description: |- + Group is the group of the referent. For example, "gateway.networking.k8s.io". + When unspecified or empty string, core API group is inferred. + maxLength: 253 + pattern: ^$|^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ + type: string + kind: + default: Secret + description: Kind is kind of the referent. For example "Secret". + maxLength: 63 + minLength: 1 + pattern: ^[a-zA-Z]([-a-zA-Z0-9]*[a-zA-Z0-9])?$ + type: string name: - description: name is unique within a namespace to reference a - secret resource. + description: Name is the name of the referent. + maxLength: 253 + minLength: 1 type: string namespace: - description: namespace defines the space within which the secret - name must be unique. + description: |- + Namespace is the namespace of the referenced object. When unspecified, the local + namespace is inferred. + + Note that when a namespace different than the local namespace is specified, + a ReferenceGrant object is required in the referent namespace to allow that + namespace's owner to accept the reference. See the ReferenceGrant + documentation for details. + + Support: Core + maxLength: 63 + minLength: 1 + pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?$ type: string + required: + - name type: object - x-kubernetes-map-type: atomic required: - image type: object diff --git a/controller/kongplugininstallation/controller.go b/controller/kongplugininstallation/controller.go index c752d1220..dc66d5700 100644 --- a/controller/kongplugininstallation/controller.go +++ b/controller/kongplugininstallation/controller.go @@ -4,9 +4,11 @@ import ( "context" "errors" "fmt" + "reflect" "github.com/samber/lo" corev1 "k8s.io/api/core/v1" + k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "oras.land/oras-go/v2/registry/remote/credentials" @@ -18,14 +20,19 @@ import ( ctrllog "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" + gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" + gatewayv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" "github.com/kong/gateway-operator/api/v1alpha1" "github.com/kong/gateway-operator/controller/kongplugininstallation/image" "github.com/kong/gateway-operator/controller/pkg/log" + "github.com/kong/gateway-operator/controller/pkg/secrets/ref" "github.com/kong/gateway-operator/pkg/utils/kubernetes" "github.com/kong/gateway-operator/pkg/utils/kubernetes/resources" ) +const kindKongPluginInstallation = gatewayv1.Kind("KongPluginInstallation") + // Reconciler reconciles a KongPluginInstallation object. type Reconciler struct { client.Client @@ -64,6 +71,15 @@ func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager) err }), ), ). + Watches( + &gatewayv1beta1.ReferenceGrant{}, + handler.EnqueueRequestsFromMapFunc(r.listReferenceGrantsForKongPluginInstallation), + builder.WithPredicates( + ref.ReferenceGrantForSecretFrom( + gatewayv1.Group(v1alpha1.SchemeGroupVersion.Group), kindKongPluginInstallation, + ), + ), + ). Complete(r) } @@ -86,21 +102,36 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu var credentialsStore credentials.Store if kpi.Spec.ImagePullSecretRef != nil { log.Trace(logger, "getting secret for KongPluginInstallation resource", kpi) - secretNN := client.ObjectKey{ - Namespace: kpi.Spec.ImagePullSecretRef.Namespace, - Name: kpi.Spec.ImagePullSecretRef.Name, + kpiNamespace := gatewayv1.Namespace(kpi.Namespace) + imagePullSecretRef := kpi.Spec.ImagePullSecretRef + ref.EnsureNamespaceInSecretRef(imagePullSecretRef, kpiNamespace) + if err := ref.DoesFieldReferenceCoreV1Secret(*imagePullSecretRef, "imagePullSecretRef"); err != nil { + return ctrl.Result{}, setStatusConditionFailedForKongPluginInstallation(ctx, r.Client, &kpi, err.Error()) + } + whyNotGrantedMsg, isReferenceGranted, refErr := ref.CheckReferenceGrantForSecret( + ctx, r.Client, &kpi, *imagePullSecretRef, + ) + if refErr != nil { + return ctrl.Result{}, fmt.Errorf("failed to resolve reference: %w", refErr) } - if secretNN.Namespace == "" { - secretNN.Namespace = req.Namespace + if !isReferenceGranted { + return ctrl.Result{}, setStatusConditionFailedForKongPluginInstallation(ctx, r.Client, &kpi, whyNotGrantedMsg) } + secretNN := client.ObjectKey{ + Namespace: string(*imagePullSecretRef.Namespace), + Name: string(imagePullSecretRef.Name), + } var secret corev1.Secret if err := r.Client.Get( ctx, secretNN, &secret, ); err != nil { - return ctrl.Result{}, setStatusConditionFailedForKongPluginInstallation(ctx, r.Client, &kpi, fmt.Sprintf("cannot retrieve secret %q, because: %s", secretNN, err)) + if k8serrors.IsNotFound(err) { + return ctrl.Result{}, setStatusConditionFailedForKongPluginInstallation(ctx, r.Client, &kpi, fmt.Sprintf("cannot retrieve secret %q, because: %s", secretNN, err)) + } + return ctrl.Result{}, fmt.Errorf("something unexpected during fetching secret %s: %w", secretNN, err) } const requiredKey = ".dockerconfigjson" @@ -179,15 +210,41 @@ func (r *Reconciler) listKongPluginInstallationsForSecret(ctx context.Context, o if kpi.Spec.ImagePullSecretRef == nil { continue } - if kpi.Spec.ImagePullSecretRef.Namespace == "" { - kpi.Spec.ImagePullSecretRef.Namespace = kpi.Namespace + ref.EnsureNamespaceInSecretRef(kpi.Spec.ImagePullSecretRef, gatewayv1.Namespace(kpi.Namespace)) + if err := ref.DoesFieldReferenceCoreV1Secret(*kpi.Spec.ImagePullSecretRef, "imagePullSecretRef"); err != nil { + continue } - if kpi.Spec.ImagePullSecretRef.Namespace == namespace && kpi.Spec.ImagePullSecretRef.Name == name { + if string(*kpi.Spec.ImagePullSecretRef.Namespace) == namespace && string(kpi.Spec.ImagePullSecretRef.Name) == name { recs = append(recs, reconcile.Request{ - NamespacedName: client.ObjectKey{ - Name: kpi.Name, - Namespace: kpi.Namespace, - }, + NamespacedName: client.ObjectKeyFromObject(&kpi), + }) + } + } + return recs +} + +func (r *Reconciler) listReferenceGrantsForKongPluginInstallation(ctx context.Context, obj client.Object) []reconcile.Request { + logger := ctrllog.FromContext(ctx) + + grant, ok := obj.(*gatewayv1beta1.ReferenceGrant) + if !ok { + logger.Error( + fmt.Errorf("unexpected object type"), + "ReferenceGrant watch predicate received unexpected object type", + "expected", "*gatewayapi.ReferenceGrant", "found", reflect.TypeOf(obj), + ) + return nil + } + var kpiList v1alpha1.KongPluginInstallationList + if err := r.Client.List(ctx, &kpiList); err != nil { + logger.Error(err, "Failed to list KongPluginInstallations in watch", "referencegrant", grant.Name) + return nil + } + var recs []reconcile.Request + for _, kpi := range kpiList.Items { + if ref.IsReferenceGrantForObj(grant, &kpi) { + recs = append(recs, reconcile.Request{ + NamespacedName: client.ObjectKeyFromObject(&kpi), }) } } diff --git a/docs/api-reference.md b/docs/api-reference.md index fdd810527..721f779a6 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -395,7 +395,7 @@ KongPluginInstallationSpec provides the information necessary to retrieve and in | Field | Description | | --- | --- | | `image` _string_ | The image is an OCI image URL for a packaged custom Kong plugin. | -| `imagePullSecretRef` _[SecretReference](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.28/#secretreference-v1-core)_ | ImagePullSecretRef is a reference to a Kubernetes Secret containing credentials necessary to pull the OCI image in Image. It must follow the format in https://kubernetes.io/docs/tasks/configure-pod-container/pull-image-private-registry. It is optional. If the image is public, omit this field. | +| `imagePullSecretRef` _[SecretObjectReference](#secretobjectreference)_ | ImagePullSecretRef is a reference to a Kubernetes Secret containing credentials necessary to pull the OCI image in Image. It must follow the format in https://kubernetes.io/docs/tasks/configure-pod-container/pull-image-private-registry. It is optional. If the image is public, omit this field. | _Appears in:_ diff --git a/test/integration/test_kongplugininstallation.go b/test/integration/test_kongplugininstallation.go index d592605b0..a03f462bf 100644 --- a/test/integration/test_kongplugininstallation.go +++ b/test/integration/test_kongplugininstallation.go @@ -6,6 +6,7 @@ import ( "testing" "time" + "github.com/google/uuid" "github.com/samber/lo" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -13,6 +14,8 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" k8stypes "k8s.io/apimachinery/pkg/types" + gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" + gatewayv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" "github.com/kong/gateway-operator/api/v1alpha1" "github.com/kong/gateway-operator/pkg/consts" @@ -89,6 +92,16 @@ func TestKongPluginInstallationEssentials(t *testing.T) { require.Equal(t, pluginExpectedContent(), recreatedCM.Data) if registryCreds := GetKongPluginImageRegistryCredentialsForTests(); registryCreds != "" { + // Create secondNamespace with K8s client to check cross-namespace capabilities. + secondNamespace := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: uuid.NewString(), + }, + } + _, err := GetClients().K8sClient.CoreV1().Namespaces().Create(GetCtx(), secondNamespace, metav1.CreateOptions{}) + require.NoError(t, err) + cleaner.Add(secondNamespace) + t.Log("update KongPluginInstallation resource to a private image") kpi, err = GetClients().OperatorClient.ApisV1alpha1().KongPluginInstallations(kpiNN.Namespace).Get(GetCtx(), kpiNN.Name, metav1.GetOptions{}) kpi.Spec.Image = registryUrl + "plugin-example-private/valid:0.1.0" @@ -100,28 +113,63 @@ func TestKongPluginInstallationEssentials(t *testing.T) { t, kpiNN, metav1.ConditionFalse, "response status code 403: denied: Unauthenticated request. Unauthenticated requests do not have permission", ) - t.Log("update KongPluginInstallation resource with credentials reference") + t.Log("update KongPluginInstallation resource with credentials reference in other namespace") kpi, err = GetClients().OperatorClient.ApisV1alpha1().KongPluginInstallations(kpiNN.Namespace).Get(GetCtx(), kpiNN.Name, metav1.GetOptions{}) - secretRef := corev1.SecretReference{Name: "kong-plugin-image-registry-credentials"} // Namespace is not specified, it will be inferred. + require.NoError(t, err) + secretRef := gatewayv1.SecretObjectReference{ + Kind: lo.ToPtr(gatewayv1.Kind("Secret")), + Namespace: lo.ToPtr(gatewayv1.Namespace(secondNamespace.Name)), + Name: "kong-plugin-image-registry-credentials", + } kpi.Spec.ImagePullSecretRef = &secretRef _, err = GetClients().OperatorClient.ApisV1alpha1().KongPluginInstallations(kpiNN.Namespace).Update(GetCtx(), kpi, metav1.UpdateOptions{}) require.NoError(t, err) + t.Log("waiting for the KongPluginInstallation resource to be reconciled and report missing ReferenceGrant for the Secret with credentials") + checkKongPluginInstallationConditions( + t, kpiNN, metav1.ConditionFalse, fmt.Sprintf("Secret %s/%s reference not allowed by any ReferenceGrant", *secretRef.Namespace, secretRef.Name), + ) + t.Log("add missing ReferenceGrant for the Secret with credentials") + refGrant := &gatewayv1beta1.ReferenceGrant{ + ObjectMeta: metav1.ObjectMeta{ + Name: "kong-plugin-image-registry-credentials", + Namespace: secondNamespace.Name, + }, + Spec: gatewayv1beta1.ReferenceGrantSpec{ + To: []gatewayv1beta1.ReferenceGrantTo{ + { + Kind: gatewayv1.Kind("Secret"), + Name: lo.ToPtr(secretRef.Name), + }, + }, + From: []gatewayv1beta1.ReferenceGrantFrom{ + { + Group: gatewayv1.Group(v1alpha1.SchemeGroupVersion.Group), + Kind: gatewayv1.Kind("KongPluginInstallation"), + Namespace: gatewayv1.Namespace(namespace.Name), + }, + }, + }, + } + _, err = GetClients().GatewayClient.GatewayV1beta1().ReferenceGrants(secondNamespace.Name).Create(GetCtx(), refGrant, metav1.CreateOptions{}) + require.NoError(t, err) + t.Log("waiting for the KongPluginInstallation resource to be reconciled and report missing Secret with credentials") checkKongPluginInstallationConditions( - t, kpiNN, metav1.ConditionFalse, fmt.Sprintf(`cannot retrieve secret "%s/%s"`, kpiNN.Namespace, secretRef.Name), + t, kpiNN, metav1.ConditionFalse, fmt.Sprintf(`cannot retrieve secret "%s/%s"`, *secretRef.Namespace, secretRef.Name), ) t.Log("add missing Secret with credentials") secret := corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Name: secretRef.Name, + Name: string(secretRef.Name), + Namespace: secondNamespace.Name, }, Type: corev1.SecretTypeDockerConfigJson, StringData: map[string]string{ ".dockerconfigjson": registryCreds, }, } - _, err = GetClients().K8sClient.CoreV1().Secrets(kpiNN.Namespace).Create(GetCtx(), &secret, metav1.CreateOptions{}) + _, err = GetClients().K8sClient.CoreV1().Secrets(secondNamespace.Name).Create(GetCtx(), &secret, metav1.CreateOptions{}) require.NoError(t, err) t.Log("waiting for the KongPluginInstallation resource to be reconciled successfully") checkKongPluginInstallationConditions(