kubernetes-sigs / controller-tools

Tools to use with the controller-runtime libraries
Apache License 2.0
738 stars 420 forks source link

validation on type alias is ignored when `GODEBUG=gotypealias=0` (Go < 1.23) #1071

Closed mtardy closed 3 weeks ago

mtardy commented 1 month ago

Here is how to repro:

  1. Create a a repro directory mkdir repro and put repro.go inside of it with:

    // +groupName=repro.io
    package repro
    
    import (
    metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
    )
    
    type Repro struct {
    metav1.TypeMeta   `json:",inline"`
    metav1.ObjectMeta `json:"metadata"`
    
    Reproducer StringAlias `json:"reproducer"`
    }
    
    // +kubebuilder:validation:MaxLength=12
    type StringAlias = string
  2. Run controller-gen with:

    go run ./cmd/controller-gen/ crd paths=./repro output:stdout

    The output should be similar to

    ---
    apiVersion: apiextensions.k8s.io/v1
    kind: CustomResourceDefinition
    metadata:
      annotations:
        controller-gen.kubebuilder.io/version: (devel)
      name: reproes.repro.io
    spec:
      group: repro.io
      names:
        kind: Repro
        listKind: ReproList
        plural: reproes
        singular: repro
      scope: Namespaced
      versions:
      - name: repro
        schema:
          openAPIV3Schema:
            properties:
              apiVersion:
                description: |-
                  APIVersion defines the versioned schema of this representation of an object.
                  Servers should convert recognized schemas to the latest internal value, and
                  may reject unrecognized values.
                  More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources
                type: string
              kind:
                description: |-
                  Kind is a string value representing the REST resource this object represents.
                  Servers may infer this from the endpoint the client submits requests to.
                  Cannot be updated.
                  In CamelCase.
                  More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds
                type: string
              metadata:
                type: object
              reproducer:
                type: string
            required:
            - metadata
            - reproducer
            type: object
        served: true
        storage: true
  3. Now bump the go directive to Go 1.23.0 in go.mod or put godebug gotypesalias=1 in the go.mod with Go 1.22:

    diff --git a/go.mod b/go.mod
    index 790a5a11..c9e19176 100644
    --- a/go.mod
    +++ b/go.mod
    @@ -1,6 +1,6 @@
    module sigs.k8s.io/controller-tools
    
    -go 1.22.0
    + go 1.23.0
  4. Run controller-gen with:

    go run ./cmd/controller-gen/ crd paths=./repro output:stdout

    The output should be similar to

    ---
    apiVersion: apiextensions.k8s.io/v1
    kind: CustomResourceDefinition
    metadata:
      annotations:
        controller-gen.kubebuilder.io/version: (devel)
      name: reproes.repro.io
    spec:
      group: repro.io
      names:
        kind: Repro
        listKind: ReproList
        plural: reproes
        singular: repro
      scope: Namespaced
      versions:
      - name: repro
        schema:
          openAPIV3Schema:
            properties:
              apiVersion:
                description: |-
                  APIVersion defines the versioned schema of this representation of an object.
                  Servers should convert recognized schemas to the latest internal value, and
                  may reject unrecognized values.
                  More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources
                type: string
              kind:
                description: |-
                  Kind is a string value representing the REST resource this object represents.
                  Servers may infer this from the endpoint the client submits requests to.
                  Cannot be updated.
                  In CamelCase.
                  More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds
                type: string
              metadata:
                type: object
              reproducer:
    +           maxLength: 12
                type: string
            required:
            - metadata
            - reproducer
            type: object
        served: true
        storage: true

Explanation: without the type Alias being its own type (Go 1.22), in localNamedToSchema, the link is missing: https://github.com/kubernetes-sigs/controller-tools/blob/1d40d792491f708409229c554022de59b90637a7/pkg/crd/schema.go#L245-L254

This patch would fix it https://github.com/cilium/controller-tools/commit/d944debcff34ea02a9506bc5911b2bb64ee61dd6.

When type Alias is its own type, it goes to: https://github.com/kubernetes-sigs/controller-tools/blob/1d40d792491f708409229c554022de59b90637a7/pkg/crd/schema.go#L255-L267

And then the link is properly made.

Should we have the patch for the version so that type Alias validation works with Go 1.22 or should we just wait for this project to use Go 1.23 and thus gotypesalias=1? Thanks!

sbueringer commented 1 month ago

I think it would be good to have this fixed. The problem is that many folks are compiling controller-gen themselves with ~ arbitrarily old Go versions :)

(I guess if we increase the Go min version in go.mod that would block them from compiling with old versions, but I think the fix would be good in any case)