Go client and CRD definition of SQLUser are inconsistent

[radoslav-tomov]

Bug Description

SQLUser CRD definiton states:

password:
description: |-
  The password for the user. Can be updated. For Postgres instances this is a Required field, unless type is set to
                  either CLOUD_IAM_USER or CLOUD_IAM_SERVICE_ACCOUNT.
oneOf:
- not:
    required:
    - valueFrom
  required:
  - value
- not:
    required:
    - value
  required:
  - valueFrom
properties:
  value:
    description: Value of the field. Cannot be used if 'valueFrom'
      is specified.
    type: string
  valueFrom:
    description: Source for the field's value. Cannot be used if 'value'
      is specified.
    properties:
      secretKeyRef:
        description: Reference to a value with the given key in the
          given Secret in the resource's namespace.
        properties:
          key:
            description: Key that identifies the value to be extracted.
            type: string
          name:
            description: Name of the Secret to extract a value from.
            type: string
        required:
        - name
        - key
        type: object

Notice, that the secretKeyRef actually is a object with key and name.
The Go struct generated for the SQLUser boils down to

type UserValueFrom struct {
	/* Reference to a value with the given key in the given Secret in the resource's namespace. */
	// +optional
	SecretKeyRef *v1alpha1.ResourceRef `json:"secretKeyRef,omitempty"`
}

and the ResourceRef

type ResourceRef struct {
	/* The external name of the referenced resource */
	External string `json:"external,omitempty"`
	/* Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names */
	Name string `json:"name,omitempty"`
	/* Namespace of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/namespaces/ */
	Namespace string `json:"namespace,omitempty"`
}

So when I try to create a SQLUser resource and attempt to reference the password from Secret I get an error.

Additional Diagnostic Information

Kubernetes Cluster Version

❯ kubectl version --short
Client Version: v1.21.2
Server Version: v1.22.4

Config Connector Version

k8s-config-connector v1.71.0

Config Connector Mode

cluster

Log Output

SQLUser.sql.cnrm.cloud.google.com \"alpha\" is invalid: spec.password.valueFrom.secretKeyRef.key: Required value", "errorVerbose": "SQLUser.sql.cnrm.cloud.google.com \"XXXX\" is invalid: spec.password.valueFrom.secretKeyRef.key: Required value

Steps to Reproduce

Steps to reproduce the issue

[mbzomowski]

Hi @radoslav-tomov, sorry to hear you're having trouble with this. I'm having some issues figuring out exactly what problem you're running into here. Would you mind providing the (redacted if necessary) yamls for both your SQLUser CR and your SecretKey CR?

Also, in what context are you using the resourceRef field here? I don't believe that's a valid field in the SQLUser CRD.

[radoslav-tomov]

@mbzomowski The problem appears when you try to create SQLUser with a password, which is stored in a secret via the go-client.

Check the Password field in the SQLUser, it boils down to the resourceRef and it is not compatible with the CRD definition. One has key and name fields, the other has name, namespace and external. Check the fix I sugested. here is the definition. SecretKeyRef should be of type, which has name and key fields, which is not the case now.

type UserPassword struct {
	/* Value of the field. Cannot be used if 'valueFrom' is specified. */
	// +optional
	Value *string `json:"value,omitempty"`

	/* Source for the field's value. Cannot be used if 'value' is specified. */
	// +optional
	ValueFrom *UserValueFrom `json:"valueFrom,omitempty"`
}

type UserValueFrom struct {
	/* Reference to a value with the given key in the given Secret in the resource's namespace. */
	// +optional
	SecretKeyRef *v1alpha1.ResourceRef `json:"secretKeyRef,omitempty"`
}

[caieo]

Hi @radoslav-tomov, this is a bug that is an oversight on our part. Our Go-Client code generation assumes that fields with the suffix Ref are a resourceRef type, but this is not necessarily true from the scenario you provided. We will look into fixing this bug on our side and sorry for the inconvenience!

[aw185176]

Echoing that this issue is useful to us as well at NCR, any ETA on a fix?

[xiaobaitusi]

Hi @aw185176 thanks for providing another data point.

Unfortunately our team hasn't gotten around to it yet, I couldn't provide a good ETA at this point. It has been tracked in our backlog and we will update this thread when we have more update.

Sorry for the inconvenience! Could you please let us know the impact of this issue? Is it currently a blocker for you? This will help us to better prioritize our tasks.

[b4nst]

This issue prevent us from using automatically generated cue definitions to validate our configuration.

[zealws]

It looks like the SecretKeyRef fields should probably be this SecretKeyReference struct instead (godocs).

Here's all the references to *v1alpha1.ResourceRef that looks like they might be wrong:

❯ git grep -r '*v1alpha1.ResourceRef' | grep Secret
pkg/clients/generated/apis/alloydb/v1alpha1/alloydbcluster_types.go:    SecretKeyRef *v1alpha1.ResourceRef `json:"secretKeyRef,omitempty"`
pkg/clients/generated/apis/bigqueryconnection/v1alpha1/bigqueryconnectionconnection_types.go:   SecretKeyRef *v1alpha1.ResourceRef `json:"secretKeyRef,omitempty"`
pkg/clients/generated/apis/bigquerydatatransfer/v1alpha1/bigquerydatatransferconfig_types.go:   SecretKeyRef *v1alpha1.ResourceRef `json:"secretKeyRef,omitempty"`
pkg/clients/generated/apis/certificatemanager/v1alpha1/certificatemanagercertificate_types.go:  SecretKeyRef *v1alpha1.ResourceRef `json:"secretKeyRef,omitempty"`
pkg/clients/generated/apis/compute/v1alpha1/computebackendbucketsignedurlkey_types.go:  SecretKeyRef *v1alpha1.ResourceRef `json:"secretKeyRef,omitempty"`
pkg/clients/generated/apis/compute/v1alpha1/computebackendservicesignedurlkey_types.go: SecretKeyRef *v1alpha1.ResourceRef `json:"secretKeyRef,omitempty"`
pkg/clients/generated/apis/compute/v1beta1/computebackendservice_types.go:      SecretKeyRef *v1alpha1.ResourceRef `json:"secretKeyRef,omitempty"`
pkg/clients/generated/apis/compute/v1beta1/computedisk_types.go:        SecretKeyRef *v1alpha1.ResourceRef `json:"secretKeyRef,omitempty"`
pkg/clients/generated/apis/compute/v1beta1/computeinstance_types.go:    SecretKeyRef *v1alpha1.ResourceRef `json:"secretKeyRef,omitempty"`
pkg/clients/generated/apis/compute/v1beta1/computesnapshot_types.go:    SecretKeyRef *v1alpha1.ResourceRef `json:"secretKeyRef,omitempty"`
pkg/clients/generated/apis/compute/v1beta1/computesslcertificate_types.go:      SecretKeyRef *v1alpha1.ResourceRef `json:"secretKeyRef,omitempty"`
pkg/clients/generated/apis/compute/v1beta1/computevpntunnel_types.go:   SecretKeyRef *v1alpha1.ResourceRef `json:"secretKeyRef,omitempty"`
pkg/clients/generated/apis/container/v1beta1/containercluster_types.go: SecretKeyRef *v1alpha1.ResourceRef `json:"secretKeyRef,omitempty"`
pkg/clients/generated/apis/datastream/v1alpha1/datastreamconnectionprofile_types.go:    SecretKeyRef *v1alpha1.ResourceRef `json:"secretKeyRef,omitempty"`
pkg/clients/generated/apis/identityplatform/v1beta1/identityplatformconfig_types.go:    SecretKeyRef *v1alpha1.ResourceRef `json:"secretKeyRef,omitempty"`
pkg/clients/generated/apis/identityplatform/v1beta1/identityplatformoauthidpconfig_types.go:    SecretKeyRef *v1alpha1.ResourceRef `json:"secretKeyRef,omitempty"`
pkg/clients/generated/apis/identityplatform/v1beta1/identityplatformtenantoauthidpconfig_types.go:      SecretKeyRef *v1alpha1.ResourceRef `json:"secretKeyRef,omitempty"`
pkg/clients/generated/apis/kms/v1alpha1/kmssecretciphertext_types.go:   SecretKeyRef *v1alpha1.ResourceRef `json:"secretKeyRef,omitempty"`
pkg/clients/generated/apis/monitoring/v1beta1/monitoringnotificationchannel_types.go:   SecretKeyRef *v1alpha1.ResourceRef `json:"secretKeyRef,omitempty"`
pkg/clients/generated/apis/monitoring/v1beta1/monitoringuptimecheckconfig_types.go:     SecretKeyRef *v1alpha1.ResourceRef `json:"secretKeyRef,omitempty"`
pkg/clients/generated/apis/networkservices/v1alpha1/networkservicesedgecachekeyset_types.go:    SecretKeyRef *v1alpha1.ResourceRef `json:"secretKeyRef,omitempty"`
pkg/clients/generated/apis/run/v1beta1/runservice_types.go:     SecretKeyRef *v1alpha1.ResourceRef `json:"secretKeyRef,omitempty"`
pkg/clients/generated/apis/secretmanager/v1beta1/secretmanagersecretversion_types.go:   SecretKeyRef *v1alpha1.ResourceRef `json:"secretKeyRef,omitempty"`
pkg/clients/generated/apis/sql/v1beta1/sqlinstance_types.go:    SecretKeyRef *v1alpha1.ResourceRef `json:"secretKeyRef,omitempty"`
pkg/clients/generated/apis/sql/v1beta1/sqluser_types.go:        SecretKeyRef *v1alpha1.ResourceRef `json:"secretKeyRef,omitempty"`
pkg/clients/generated/apis/storagetransfer/v1beta1/storagetransferjob_types.go: SecretKeyRef *v1alpha1.ResourceRef `json:"secretKeyRef,omitempty"`

I haven't tried this, but I think a work-around for this would be to use the kube unstructured API instead, but you lose out on type safety using the Go SDK, which is problematic.

Our Go-Client code generation assumes that fields with the suffix Ref are a resourceRef type

I don't know exactly how the generator for this code works, but if it's picking up the Ref name suffix and assuming the associated type should be *v1alpha1.ResourceRef, would changing the name of those fields to SecretKeyReference be enough to fix it?

If that's not enough, you could rename the fields SecretKeyLocator or something else instead (pick your favorite synonym).

[zealws]

Also, if the Ref suffix is causing the problem, then every reference to SecretKeyRef in the codebase would be broken by extension, since they'd all be using the wrong type.

Spot-checking that assumption...

• There's only 1 reference to the correct v1alpha1.SecretKeyReference struct in the codebase (in k8s.GetSecretVal).
• k8s.GetSecretVal is referenced in 4 places.
• all 4 calls to k8s.GetSecretVal load unstructured data to get a v1alpha1.SecretKeyReference, rather than using the Go SDK resource types which contain a v1alpha1.SecretKeyReference.

So there's no code at all in this codebase that uses a Go SDK resource type containing a v1alpha1.SecretKeyReference.

I think that's a pretty good indicator that all the go resource types which contain SecretKeyRef fields are unusable in the current version of the Go SDK (at least, if you want to use the SecretKeyRef field).

That's 26 resource types (from the git grep -r '*v1alpha1.ResourceRef' | grep Secret in the previous comment), for reference.

[diviner524]

@zealws Thanks for the research! Yes it is coming from the incorrect assumption here:

https://github.com/GoogleCloudPlatform/k8s-config-connector/blob/master/scripts/generate-go-crd-clients/generate-types-file.go#L327

We apologize that we may not be able to address the issue immediately due to current capacity limitations. However, if you're interested in contributing to fix the issue, we would be happy to provide some guidance on how to generate and validate the Go client. Please let us know if you'd like to get involved, and we'll share some pointers to help you get started!