Open czeslavo opened 4 months ago
@mheap After a discussion with @lahabana I created this issue to have it properly tracked in GH. I added an ad-hoc proposed solution, but I think we will need to iterate over it with a proper review.
BTW Should we create KGO 1.4 or "API summit" milestone and assign the issue to it?
I'd make this a GatewayConfiguration
in addition to Konnect specific data planes I think. This allows us to use it with both Gateway
and DataPlane
kind: KonnectConfiguration
apiVersion: gateway-operator.konghq.com/v1beta1
metadata:
name: kong
namespace: default
spec:
konnectOptions:
controlPlaneId: YOUR_CP_ID
controlPlaneRegion: us|eu|au
prefix: YOUR_PREFIX
tlsSecretName: konnect-client-tls
controlPlaneOptions:
deployment:
podTemplateSpec:
spec:
containers:
- name: controller
image: kong/kubernetes-ingress-controller:3.1.3
env:
- name: CONTROLLER_LOG_LEVEL
value: debug
dataPlaneOptions:
deployment:
podTemplateSpec:
spec:
containers:
- name: proxy
image: kong/kong-gateway:3.7.0.0
apiVersion: gateway-operator.konghq.com/v1beta1
kind: DataPlane
metadata:
name: dataplane-example
namespace: kong
spec:
deployment:
podTemplateSpec:
spec:
konnectOptions:
controlPlaneId: YOUR_CP_ID
controlPlaneRegion: us|eu|au
prefix: YOUR_PREFIX
tlsSecretName: konnect-client-tls
I believe we can sync on this (this issue is still unassigned so not sure if anyone is working on this).
DataPlane
's spec.deployment.podTemplateSpec
(or anything nested underneath it) because that's a type that we do not control https://github.com/Kong/gateway-operator/blob/3876430e09e61edce58bd8464989e33236bd1872/api/v1beta1/shared_types.go#L35. We wouldn't like to do it anyway as that's directly translated into an "overlay" of sorts and merged using strategic merge patch on top of the generated spec.We need to think carefully about the fields and what type we want to use for configuration. Working on the design for #370 I came up with KonnectConfiguration CRD that would be referenced by Konnect entitity types. Roughly:
// +genclient
// +genclient:nonNamespaced
// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
// +kubebuilder:object:root=true
// +kubebuilder:validation:XValidation:rule="self.token.startsWith('spat_') || self.token.startsWith('kpat_')", message="Konnect tokens have to start with spat_ or kpat_"
// KonnectConfiguration is the Schema for the Konnect configuration type.
type KonnectConfiguration struct {
metav1.TypeMeta `json:",inline"`
metav1.ObjectMeta `json:"metadata,omitempty"`
// Token is the Konnect token used to authenticate with the Konnect API.
// +kubebuilder:validation:Required
Token string `json:"token"`
// ServerURL is the URL of the Konnect server.
// +kubebuilder:validation:Enum=us.api.konghq.com;eu.api.konghq.com;au.api.konghq.com
// +kubebuilder:validation:Required
ServerURL string `json:"serverURL"`
}
type KonnectConfigurationRef struct {
// Name is the name of the KonnectConfiguration resource.
// +kubebuilder:validation:Required
Name string `json:"name"`
}
This can then be referenced in entity types like so:
// KonnectControlPlane is the Schema for the konnectcontrolplanes API.
type KonnectControlPlane struct {
metav1.TypeMeta `json:",inline"`
metav1.ObjectMeta `json:"metadata,omitempty"`
Spec KonnectControlPlaneSpec `json:"spec,omitempty"`
Status KonnectEntityStatus `json:"status,omitempty"`
}
type KonnectControlPlaneSpec struct {
// Spec ...
KonnectConfigurationRef KonnectConfigurationRef `json:"konnectConfigurationRef,omitempty"`
}
This is important because we don't want to confuse users and the proposed KonnectConfiguration CRD serves a slightly different purpose than the one I propose.
My initial proposal written down in the issue description was rather an ad-hoc base so we can have something to discuss. I agree with @mheap that it would be better to have some struct that we could use both in Gateway
and DataPlane
(as @pmalek noted, it won't be possible to change podTemplateSpec
, but we might add it as a separate DP option I believe).
As it comes to naming, I think we have to make sure that we do not end up with ambiguous KonnectConfiguration
-like CRDs/structs that might mean almost anything. I'd aim for something more specific, e.g. DataPlaneKonnectOptions
for the struct that will include all params needed for DP-Konnect comms, and KonnectAPIAuthConfiguration
for the CRD @pmalek proposed for KGO-Konnect authentication purposes.
I believe we can sync on this (this issue is still unassigned so not sure if anyone is working on this).
👍 I'm happy to sync on this next week. I don't think anyone's working on this issue right now.
KonnectAPIAuthConfiguration
for the CRD @pmalek proposed for KGO-Konnect authentication purposes.
I like this. And putting it inside DataPlaneKonnectOptions
or anything else that will require to authenticate against Konnect seems like a good approach.
Problem Statement
Currently, to deploy
DataPlane
s that will sync with Konnect Control Plane, one has to define a very verbose PodTemplateSpec patch that will define all the required environment variables Kong Gateway needs for that purpose:It's not the best user experience as there's a lot of repetition and it's easy to make a mistake.
We should make DataPlane natively integrated with Konnect by extending the CRD with a Konnect-specific spec section.
Proposed Solution
Extend
DataPlaneOptions
withKonnect
section as below:This will reduce the repetition and allow rigid validation of the input based on the defined schema.
Acceptance Criteria