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

CustomResourceDefinition is invalid at kubernetes 1.18 #3235

Closed
f41gh7 opened this issue Jun 14, 2020 · 21 comments
Closed

CustomResourceDefinition is invalid at kubernetes 1.18 #3235

f41gh7 opened this issue Jun 14, 2020 · 21 comments
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Milestone

Comments

@f41gh7
Copy link

f41gh7 commented Jun 14, 2020

Bug Report

What did you do?

create a new operator and add it api into it:

operator-sdk new test-operator --repo=test
operator-sdk add api --api-version=app.example.com/v1alpha1 --kind=AppService

Add init containers to pkg/apis/v1alpha1/appservice_types.go

// AppServiceSpec defines the desired state of AppService
type AppServiceSpec struct {
 // INSERT ADDITIONAL SPEC FIELDS - desired state of cluster
 // Important: Run "operator-sdk generate k8s" to regenerate code after modifying this file
 // Add custom validation using kubebuilder tags: https://book-v1.book.kubebuilder.io/beyond_basics/generating_crd.html
 InitContainers []v1.Container `json:"initContainers"`
}

generate new crd and apply it:

 operator-sdk generate crds
 kubectl apply -f deploy/crds/app.example.com_appservices_crd.yaml 

get error:

The CustomResourceDefinition "appservices.app.example.com" is invalid: spec.validation.openAPIV3Schema.properties[spec].properties[initContainers].items.properties[ports].items.properties[protocol].default: Required value: this property is in x-kubernetes-list-map-keys, so it must have a default or be a required property

What did you expect to see?

add CRD without an error

What did you see instead? Under which circumstances?
error:

The CustomResourceDefinition "appservices.app.example.com" is invalid: spec.validation.openAPIV3Schema.properties[spec].properties[initContainers].items.properties[ports].items.properties[protocol].default: Required value: this property is in x-kubernetes-list-map-keys, so it must have a default or be a required property

Environment

  • operator-sdk version:

1.18.1

  • go version:

1.13.4

  • Kubernetes version information:

1.18.2

  • Kubernetes cluster kind:

  • Are you writing your operator in ansible, helm, or go?

Possible Solution

  1. Add a default value to CRD
  2. replace []Container description at CRD with type: Array, type Object

Additional context

This problem is related to new 1.18 validations:

kubernetes-sigs/kubebuilder#1544

protocol - must have default value TCP:

default: TCP

                    ports:
                      description: List of ports to expose from the container. Exposing
                        a port here gives the system additional information about
                        the network connections a container uses, but is primarily
                        informational. Not specifying a port here DOES NOT prevent
                        that port from being exposed. Any port which is listening
                        on the default "0.0.0.0" address inside a container will be
                        accessible from the network. Cannot be updated.
                      items:
                        description: ContainerPort represents a network port in a
                          single container.
                        properties:
                          containerPort:
                            description: Number of port to expose on the pod's IP
                              address. This must be a valid port number, 0 < x < 65536.
                            format: int32
                            type: integer
                          hostIP:
                            description: What host IP to bind the external port to.
                            type: string
                          hostPort:
                            description: Number of port to expose on the host. If
                              specified, this must be a valid port number, 0 < x <
                              65536. If HostNetwork is specified, this must match
                              ContainerPort. Most containers do not need this.
                            format: int32
                            type: integer
                          name:
                            description: If specified, this must be an IANA_SVC_NAME
                              and unique within the pod. Each named port in a pod
                              must have a unique name. Name for the port that can
                              be referred to by services.
                            type: string
                          protocol:
                            description: Protocol for port. Must be UDP, TCP, or SCTP.
                              Defaults to "TCP".
                            type: string
                        required:
                        - containerPort
                        type: object
                      type: array
                      x-kubernetes-list-map-keys:
                      - containerPort
                      - protocol
                      x-kubernetes-list-type: map
@tengqm
Copy link
Contributor

tengqm commented Jun 15, 2020

wow, this looks like a sig-apimachinery issue.

@estroz
Copy link
Member

estroz commented Jun 15, 2020

Yeah this may be an upstream bug if kubebuilder is similarly affected.

/triage needs-information
/kind bug

@openshift-ci-robot openshift-ci-robot added triage/needs-information Indicates an issue needs more information in order to work on it. kind/bug Categorizes issue or PR as related to a bug. labels Jun 15, 2020
@estroz
Copy link
Member

estroz commented Jun 15, 2020

/assign

@kensipe
Copy link
Member

kensipe commented Jun 15, 2020

@estroz it took some hunting... but I found the open issue around this: kubernetes-sigs/controller-tools#444

There is good detail discussion on this thread: kubernetes/kubernetes#91395

the core of it is:

So, the only missing piece is that core types lack any way to indicate default values.

Indicating defaults in struct annotations has been on the backlog for a while (xref #25460). Note that not all of our defaults can be expressed declaratively (some defaults vary depending on values of other fields, for example), but as far as I'm aware the ones used as keys in list-map items are constant. A KEP proposing a specific approach would be welcome.

@f41gh7
Copy link
Author

f41gh7 commented Jun 18, 2020

It seems like removing core Kubernetes types scheme from crd open API is only one working solution for now. Probably it can be solved with some kubebuilder api marker in the future.

Should i address this issue to kubebuilder?

@kensipe
Copy link
Member

kensipe commented Jun 23, 2020

/cc @jennybuckley @jpbetz

@estroz
Copy link
Member

estroz commented Aug 8, 2020

Seems to be fixed by kubernetes-sigs/controller-tools#440 kubernetes-sigs/controller-tools#480. We’ll need to bump c-t in kubebuilder then bump kubebuilder for this to be fixed.

@a1046149428
Copy link

Is there any update for this issue?

@f41gh7
Copy link
Author

f41gh7 commented Sep 17, 2020

I've decided to remove inherited open api scheme for kubernetes built-in objects from my crds.

My motivation - it makes crd heavier. So, you cannot use kubectl apply -f crd.yaml for crd spec with few v1.Container fields. And some integrations breaks as well (operator-hub tests for instance).

@jberkhahn jberkhahn modified the milestones: v1.1.0, v1.2.0 Sep 30, 2020
@jberkhahn jberkhahn removed the triage/needs-information Indicates an issue needs more information in order to work on it. label Sep 30, 2020
amuraru added a commit to adobe/zookeeper-operator that referenced this issue Oct 6, 2020
This is an workaround for k8s 1.18 crd to remove
the required field protocol


See:
- datastax/cass-operator#132
- kubernetes-sigs/controller-tools#444
- operator-framework/operator-sdk#3235
amuraru added a commit to adobe/zookeeper-operator that referenced this issue Oct 7, 2020
This is an workaround for k8s 1.18 crd to remove
the required field protocol


See:
- datastax/cass-operator#132
- kubernetes-sigs/controller-tools#444
- operator-framework/operator-sdk#3235
@gmtstephane
Copy link

Hello, still nothing new ?
It is impossible to add Any k8s resources in our CRD, so it make creating operator quiet useless if we cannot handle its managed ressources directly in his spec.
The workaround is to fetch the ressources every time in the reconcile loop.

I would define this as a major issue.
Thanks

@estroz
Copy link
Member

estroz commented Oct 30, 2020

Workaround for post-v1.0 projects:

# Set CRD version so v1beta1 versions are generated
sed -i 's/\(CRD_OPTIONS ?= "crd:\)/\1crdVersions={v1beta1},/g' Makefile
# If you have any webhooks:
sed -i 's/\(+kubebuilder:webhook:\)/\1webhookVersions={v1beta1},admissionReviewVersions={v1beta1},/g' api/<version>/<kind>_webhook.go
# Then install the controller-gen binary containing the fix
rm -f $(command -v controller-gen)
mkdir tmp && pushd tmp && go mod init tmp
go get sigs.k8s.io/controller-tools/cmd/controller-gen@6fa696de4772fb44595d25355946254885a202df
popd tmp && rm -rf tmp
make manifests

@gmtstephane
Copy link

gmtstephane commented Nov 1, 2020

Hi, i tried your fix, without results, with a simple example :

// EcpmSpec defines the desired state of Ecpm
type EcpmSpec struct {	
	Foo     string            `json:"foo,omitempty"`
	MariaDB appsv1.Deployment `json:"mariaDB,omitempty"`
}
// EcpmStatus defines the observed state of Ecpm
type EcpmStatus struct {
}

Following your instructions, My makefile is like :
CRD_OPTIONS ?= "crd:crdVersions={v1beta1},crdVersions={v1beta1},trivialVersions=true"

and my controller-gen version :

➜  operator-demo controller-gen --version
Version: v0.4.1-0.20200901161351-4f077b9ee59f

When i try to install the crd :

Warning: apiextensions.k8s.io/v1beta1 CustomResourceDefinition is deprecated in v1.16+, unavailable in v1.22+; use apiextensions.k8s.io/v1 CustomResourceDefinition
The CustomResourceDefinition "ecpms.*****.*****.*****" is invalid: 
* spec.validation.openAPIV3Schema.properties[spec].properties[mariaDB].properties[spec].properties[template].properties[spec].properties[containers].items.properties[ports].items.properties[protocol].default: Required value: this property is in x-kubernetes-list-map-keys, so it must have a default or be a required property
* spec.validation.openAPIV3Schema.properties[spec].properties[mariaDB].properties[spec].properties[template].properties[spec].properties[initContainers].items.properties[ports].items.properties[protocol].default: Required value: this property is in x-kubernetes-list-map-keys, so it must have a default or be a required property

Thanks.

Edit : Same error if i use v1 instead of v1beta1.

@estroz
Copy link
Member

estroz commented Nov 2, 2020

@gmtstephane I linked the wrong commit in the instructions, try 6fa696de4772fb44595d25355946254885a202df (updated above).

@gmtstephane
Copy link

@gmtstephane I linked the wrong commit in the instructions, try 6fa696de4772fb44595d25355946254885a202df (updated above).
Working Thanks !

@hensur
Copy link
Contributor

hensur commented Nov 3, 2020

Workaround for 0.18.x projects:
I just bumped the controller-tools dependency and built a new binary.

The generated manifests have the default field set.

Required changes: hensur@ffa2f2c

@estroz
Copy link
Member

estroz commented Nov 6, 2020

Since this is fixed upstream but isn't available because go/v3-alpha isn't hasn't been added to operator-sdk init yet, so I'm bumping this to v1.3

@estroz
Copy link
Member

estroz commented Dec 1, 2020

This is still an upstream issue, see kubernetes-sigs/controller-tools#529.

I'm moving this to the backlog until this is resolved in controller-gen once and for all, and a patched v0.3 version is created for go/v2 projects.

@estroz estroz modified the milestones: v1.3.0, Backlog Dec 1, 2020
@illeatmyhat
Copy link

illeatmyhat commented Feb 10, 2021

FYI for those with recent projects, controller-gen 4.1 has a workaround

@jpbetz
Copy link

jpbetz commented Feb 10, 2021

cc @apelisse for the map key defaulting aspect of this

@apelisse
Copy link

We fixed this in Kubernetes 1.19. Let me know if I can help.

@f41gh7
Copy link
Author

f41gh7 commented Feb 10, 2021

For me it works, thanks!

@f41gh7 f41gh7 closed this as completed Feb 10, 2021
mnencia added a commit to cloudnative-pg/cloudnative-pg that referenced this issue Apr 1, 2022
This patch adds a new parameter to the CRD named `podTemplate`, containing a
template that is used to create Pods for PostgreSQL instances. This feature is
useful to fine-grain configure things that don't have a direct field in
CRD, like NodeAffinity.

The patch also updates the version of `controllen-gen` we are using the latest version
because the previous one had a bug preventing the generation of CRDs where PodTemplate
is used. More information can be found here:

operator-framework/operator-sdk#3235

Co-authored-by: Leonardo Cecchi <leonardo.cecchi@2ndquadrant.it>
Co-authored-by: Jonathan Gonzalez V <jonathan.gonzalez@2ndquadrant.com>
Co-authored-by: Marco Nenciarini <marco.nenciarini@2ndquadrant.it>
Co-authored-by: Gabriele Bartolini <gabriele.bartolini@2ndquadrant.it>
mnencia added a commit to cloudnative-pg/cloudnative-pg that referenced this issue Apr 5, 2022
This patch adds a new parameter to the CRD named `podTemplate`, containing a
template that is used to create Pods for PostgreSQL instances. This feature is
useful to fine-grain configure things that don't have a direct field in
CRD, like NodeAffinity.

The patch also updates the version of `controllen-gen` we are using the latest version
because the previous one had a bug preventing the generation of CRDs where PodTemplate
is used. More information can be found here:

operator-framework/operator-sdk#3235

Co-authored-by: Leonardo Cecchi <leonardo.cecchi@2ndquadrant.it>
Co-authored-by: Jonathan Gonzalez V <jonathan.gonzalez@2ndquadrant.com>
Co-authored-by: Marco Nenciarini <marco.nenciarini@2ndquadrant.it>
Co-authored-by: Gabriele Bartolini <gabriele.bartolini@2ndquadrant.it>
mnencia added a commit to cloudnative-pg/cloudnative-pg that referenced this issue Apr 5, 2022
This patch adds a new parameter to the CRD named `podTemplate`, containing a
template that is used to create Pods for PostgreSQL instances. This feature is
useful to fine-grain configure things that don't have a direct field in
CRD, like NodeAffinity.

The patch also updates the version of `controllen-gen` we are using the latest version
because the previous one had a bug preventing the generation of CRDs where PodTemplate
is used. More information can be found here:

operator-framework/operator-sdk#3235

Co-authored-by: Leonardo Cecchi <leonardo.cecchi@2ndquadrant.it>
Co-authored-by: Jonathan Gonzalez V <jonathan.gonzalez@2ndquadrant.com>
Co-authored-by: Marco Nenciarini <marco.nenciarini@2ndquadrant.it>
Co-authored-by: Gabriele Bartolini <gabriele.bartolini@2ndquadrant.it>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Projects
None yet
Development

No branches or pull requests