cdk8s-team / cdk8s-core

Define Kubernetes native apps and abstractions using object-oriented programming
Apache License 2.0
64 stars 26 forks source link

cdk8s is not quoting strings automatically #2565

Open achanda opened 1 year ago

achanda commented 1 year ago

Description of the bug:

I am using cdk8s using Go to manage CloudnativePG clusters. The generated yaml has some config parameters without quotes.

Reproduction Steps:

package main

import (
    "fmt"
    "strconv"

    "code.cfops.it/k8s-db-clusters/pgclusters/imports/postgresqlcnpgio"
    "github.com/aws/constructs-go/constructs/v10"
    "github.com/aws/jsii-runtime-go"
    "github.com/cdk8s-team/cdk8s-core-go/cdk8s/v2"
)

type PGClusterProps struct {
    name                                 string
    namespace                            string
    instances                            int64
    storageSize                          string
    postgresCpuRequest                   int
    postgresMemoryRequest                int     // Gi
    postgresConfigSharedBuffers          int     // GB
    postgresConfigWorkMem                int     // kB
    postgresConfigMaintenanceWorkMem     int     // GB
    postgresConfigEffectiveIoConcurrency int     // no unit
    postgresConfigEffectiveCacheSize     int     // GB
    postgresConfigCpuTupleCost           float64 // no unit
}

func NewPGCluster(scope constructs.Construct, id string, props *PGClusterProps) cdk8s.Chart {
    chart := cdk8s.NewChart(scope, jsii.String(id), nil)
        postgresqlcnpgio.NewCluster(chart, jsii.String(id+"cluster"), &postgresqlcnpgio.ClusterProps{
        Metadata: &cdk8s.ApiObjectMetadata{
            Name:      jsii.String(props.name),
            Namespace: jsii.String(props.namespace),
        },
        Spec: &postgresqlcnpgio.ClusterSpec{
            Instances:             jsii.Number(props.instances),
            PrimaryUpdateStrategy: postgresqlcnpgio.ClusterSpecPrimaryUpdateStrategy_UNSUPERVISED,
            Storage: &postgresqlcnpgio.ClusterSpecStorage{
                Size: jsii.String(props.storageSize),
            },
            Bootstrap: &postgresqlcnpgio.ClusterSpecBootstrap{
                Initdb: &postgresqlcnpgio.ClusterSpecBootstrapInitdb{
                    Database:      jsii.String("app"),
                    Owner:         jsii.String("app"),
                    DataChecksums: jsii.Bool(true),
                },
            },
            Postgresql: &postgresqlcnpgio.ClusterSpecPostgresql{
                Parameters: &map[string]*string{
                    "cpu_tuple_cost":                   jsii.String(strconv.FormatFloat(props.postgresConfigCpuTupleCost, 'f', -1, 64)),
                    "checkpoint_completion_target":     jsii.String("0.9"),
                    "bgwriter_lru_maxpages":            jsii.String("1000"),
                    "bgwriter_delay":                   jsii.String("10ms"),
                    "shared_buffers":                   jsii.String(strconv.Itoa(props.postgresConfigSharedBuffers) + "GB"),
                    "effective_cache_size":             jsii.String(strconv.Itoa(props.postgresConfigEffectiveCacheSize) + "GB"),
                    "maintenance_work_mem":             jsii.String(strconv.Itoa(props.postgresConfigMaintenanceWorkMem) + "GB"),
                    "wal_buffers":                      jsii.String("16MB"),
                    "wal_log_hints":                    jsii.String("on"),
                    "default_statistics_target":        jsii.String("100"),
                    "random_page_cost":                 jsii.String("1.1"),
                    "effective_io_concurrency":         jsii.String(strconv.Itoa(props.postgresConfigEffectiveIoConcurrency)),
                    "work_mem":                         jsii.String(strconv.Itoa(props.postgresConfigWorkMem) + "kB"),
                    "min_wal_size":                     jsii.String("1GB"),
                    "max_wal_size":                     jsii.String("4GB"),
                    "max_worker_processes":             jsii.String("10"),
                    "max_parallel_workers_per_gather":  jsii.String("4"),
                    "max_parallel_workers":             jsii.String("10"),
                    "max_parallel_maintenance_workers": jsii.String("4"),
                    "log_checkpoints":                  jsii.String("on"),
                    "log_connections":                  jsii.String("on"),
                    "log_disconnections":               jsii.String("on"),
                    "log_lock_waits":                   jsii.String("on"),
                    "log_temp_files":                   jsii.String("0"),
                    "log_autovacuum_min_duration":      jsii.String("0"),
                    "log_error_verbosity":              jsii.String("default"),
                    "log_line_prefix":                  jsii.String(fmt.Sprintf("%s time=%%m,pid=%%p,user=%%u,db=%%d,client=%%r,appname=%%a,vid=%%v,xid=%%x ", props.name)),
                },
            },
        },
    })

    return chart
}

func main() {
    app := cdk8s.NewApp(nil)
    NewPGCluster(app, "mytestcluster", &PGClusterProps{
        name:                  "mytestcluster",
        namespace:             "cnpg-clusters",
        instances:             3,
        storageSize:           "10Gi",
        postgresCpuRequest:    5,
        postgresMemoryRequest: 32, // in Gi
    })
    app.Synth()
}

This produces the following yaml

apiVersion: postgresql.cnpg.io/v1
kind: Cluster
metadata:
  name: mytestcluster
  namespace: cnpg-clusters
spec:
  bootstrap:
    initdb:
      dataChecksums: true
      database: app
      owner: app
  instances: 3
  postgresql:
    parameters:
      bgwriter_delay: 10ms
      bgwriter_lru_maxpages: "1000"
      checkpoint_completion_target: "0.9"
      cpu_tuple_cost: "0.03"
      default_statistics_target: "100"
      effective_cache_size: 0GB
      effective_io_concurrency: "300"
      log_autovacuum_min_duration: "0"
      log_checkpoints: "on"
      log_connections: "on"
      log_disconnections: "on"
      log_error_verbosity: default
      log_line_prefix: "mytestcluster time=%m,pid=%p,user=%u,db=%d,client=%r,appname=%a,vid=%v,xid=%x "
      log_lock_waits: "on"
      log_temp_files: "0"
      maintenance_work_mem: 1GB
      max_parallel_maintenance_workers: "4"
      max_parallel_workers: "10"
      max_parallel_workers_per_gather: "4"
      max_wal_size: 4GB
      max_worker_processes: "10"
      min_wal_size: 1GB
      random_page_cost: "1.1"
      shared_buffers: 10GB
      wal_buffers: 16MB
      wal_log_hints: "on"
      work_mem: 0kB
  primaryUpdateStrategy: unsupervised
  storage:
    size: 10Gi

Some postgres params are quoted while some are not. This does not work since the cnpg operator expects all params to be strings.

Error Log:

There are no errors

Environment:

This is :bug: Bug Report

heckler1 commented 11 months ago

Not all YAML strings must be quoted, anything that can't be interpreted as another type will be unquoted most of the time. Is there a specific error that you're getting when trying to use these outputted manifests?

A quick check of the above manifest looks like all of those parameter values are proper strings.

iliapolo commented 3 months ago

This seems like an issue the cnpg operator because the YAML is completely valid, and all parameters are indeed strings.

Having said that, since a user can manually quote (even if to overcome a third-party bug), we should also allow this. Right now i'm not sure its possible.