Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Native support for Konnect integration in DataPlane #203

Closed
1 task
czeslavo opened this issue Apr 23, 2024 · 5 comments · Fixed by #714
Closed
1 task

Native support for Konnect integration in DataPlane #203

czeslavo opened this issue Apr 23, 2024 · 5 comments · Fixed by #714
Assignees
Milestone

Comments

@czeslavo
Copy link
Contributor

czeslavo commented Apr 23, 2024

Problem Statement

Currently, to deploy DataPlanes 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:

apiVersion: gateway-operator.konghq.com/v1beta1
kind: DataPlane
metadata:
  name: dataplane-example
  namespace: kong
spec:
  deployment:
    podTemplateSpec:
      spec:
        containers:
        - name: proxy
          image: kong/kong-gateway:3.6.1.3
          env:
            - name: KONG_ROLE
              value: data_plane
            - name: KONG_DATABASE
              value: "off"
            - name: KONG_CLUSTER_MTLS
              value: pki
            - name: KONG_CLUSTER_CONTROL_PLANE
              value: YOUR_CP_ID.us.cp0.konghq.com:443
            - name: KONG_CLUSTER_SERVER_NAME
              value: YOUR_CP_ID.us.cp0.konghq.com
            - name: KONG_CLUSTER_TELEMETRY_ENDPOINT
              value: YOUR_CP_ID.us.tp0.konghq.com:443
            - name: KONG_CLUSTER_TELEMETRY_SERVER_NAME
              value: YOUR_CP_ID.us.tp0.konghq.com
            - name: KONG_CLUSTER_CERT
              value: /etc/secrets/kong-cluster-cert/tls.crt
            - name: KONG_CLUSTER_CERT_KEY
              value: /etc/secrets/kong-cluster-cert/tls.key
            - name: KONG_LUA_SSL_TRUSTED_CERTIFICATE
              value: system
            - name: KONG_KONNECT_MODE
              value: "on"
            - name: KONG_VITALS
              value: "off"
          volumeMounts:
            - name: cluster-certificate
              mountPath: /var/cluster-certificate
            - name: kong-cluster-cert
              mountPath: /etc/secrets/kong-cluster-cert/
              readOnly: true
        volumes:
          - name: cluster-certificate
          - name: kong-cluster-cert
            secret:
              secretName: kong-cluster-cert
              defaultMode: 420

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 with Konnect section as below:

// DataPlaneOptions defines the information specifically needed to
// deploy the DataPlane.
type DataPlaneOptions struct {
        // ...

	// +optional
	Konnect *KonnectOptions `json:"konnect,omitempty"`
}

// DataPlaneKonnectKind defines the kind/role of the DataPlane deployment.
type DataPlaneKonnectKind string

const (
	ManagedByCloudControlPlaneDataPlaneKonnectKind DataPlaneKonnectKind = "managed-by-cloud-control-plane"

	// Note: just for future compatibility if we decide to implement Gateway (KIC + DPs) with Konnect integration as well.
	ManagedByIngressControllerPlaneDataPlaneKonnectKind = "managed-by-ingress-controller"
)

type KonnectOptions struct {
	// Kind defines the kind of the DataPlane deployment, which determines how it will be configured.
	Kind DataPlaneKonnectKind `json:"kind"`

	// ControlPlaneID is the identifier of the Konnect Control Plane.
	ControlPlaneID string `json:"controlPlaneID"`

	// ControlPlaneRegion is the region of the Konnect Control Plane.
	// If not set, 'us' is used as the default region.
	// +optional
	ControlPlaneRegion *string `json:"controlPlaneRegion,omitempty"`

	// ClusterCertificate is a name of the Secret containing the Konnect Control Plane's cluster certificate.
	ClusterCertificate string `json:"clusterCertificate"`
}

This will reduce the repetition and allow rigid validation of the input based on the defined schema.

Acceptance Criteria

  • As a Konnect-hosted Control Plane user who wants to deploy DataPlanes in Kubernetes using KGO, I'm provided with an API that will allow me to configure those with minimal information provided (CP ID, cluster certificate secret name), so I do not have to know what exact environment variables need to be adjusted in their Deployments
@czeslavo
Copy link
Contributor Author

@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?

@lahabana lahabana added this to the KGO v1.4.x milestone Jun 24, 2024
@lahabana lahabana assigned pmalek and unassigned pmalek Jun 24, 2024
@lahabana lahabana added the area/feature New feature or request label Jun 27, 2024
@mheap
Copy link
Member

mheap commented Jun 28, 2024

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

@pmalek
Copy link
Member

pmalek commented Jun 28, 2024

I believe we can sync on this (this issue is still unassigned so not sure if anyone is working on this).

  1. @mheap we can't use DataPlane's spec.deployment.podTemplateSpec (or anything nested underneath it) because that's a type that we do not control

    PodTemplateSpec *corev1.PodTemplateSpec `json:"podTemplateSpec,omitempty"`
    . 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.

  2. We need to think carefully about the fields and what type we want to use for configuration. Working on the design for ☔ Konnect managed entities #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.

@czeslavo
Copy link
Contributor Author

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.

@pmalek
Copy link
Member

pmalek commented Jun 28, 2024

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.

@mlavacca mlavacca self-assigned this Jul 9, 2024
@mlavacca mlavacca removed their assignment Sep 13, 2024
@mlavacca mlavacca self-assigned this Sep 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants