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

Enable CRD validation generation for Helm and Ansible #3770

Closed
camilamacedo86 opened this issue Aug 20, 2020 · 16 comments
Closed

Enable CRD validation generation for Helm and Ansible #3770

camilamacedo86 opened this issue Aug 20, 2020 · 16 comments
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. needs discussion
Milestone

Comments

@camilamacedo86
Copy link
Contributor

camilamacedo86 commented Aug 20, 2020

Feature Request

Is your feature request related to a problem? Please describe.

When we scaffold a project using a chart for example, the CRs will be automatically created with the chart values but the CRD will not be created with these data.

For Go, the CRDs are updated when we run make manifests via controller-gen and because of this, the projects pass by default in the scorecard-test olm-crds-have-validation which is not achievable with Helm or Ansible.

For reference using helm operator-sdk-sample:

$ cd ~/devel/operator-sdk-samples/helm/memcached-operator
$ operator-sdk scorecard ./bundle --selector=suite=olm

SNIP...
--------------------------------------------------------------------------------
Image:      quay.io/operator-framework/scorecard-test:master
Entrypoint: [scorecard-test olm-crds-have-validation]
Labels:
	"suite":"olm"
	"test":"olm-crds-have-validation-test"
Results:
	Name: olm-crds-have-validation
	State: fail

	Suggestions:
		Add CRD validation for spec field `metrics` in Memcached/v1alpha1
		Add CRD validation for spec field `podAnnotations` in Memcached/v1alpha1
		Add CRD validation for spec field `updateStrategy` in Memcached/v1alpha1
		Add CRD validation for spec field `extraContainers` in Memcached/v1alpha1
		Add CRD validation for spec field `image` in Memcached/v1alpha1
		Add CRD validation for spec field `kind` in Memcached/v1alpha1
		Add CRD validation for spec field `pdbMinAvailable` in Memcached/v1alpha1
		Add CRD validation for spec field `replicaCount` in Memcached/v1alpha1
		Add CRD validation for spec field `securityContext` in Memcached/v1alpha1
		Add CRD validation for spec field `tolerations` in Memcached/v1alpha1
		Add CRD validation for spec field `affinity` in Memcached/v1alpha1
		Add CRD validation for spec field `memcached` in Memcached/v1alpha1
		Add CRD validation for spec field `resources` in Memcached/v1alpha1
		Add CRD validation for spec field `serviceAnnotations` in Memcached/v1alpha1
		Add CRD validation for spec field `AntiAffinity` in Memcached/v1alpha1
		Add CRD validation for spec field `nodeSelector` in Memcached/v1alpha1
		Add CRD validation for spec field `extraVolumes` in Memcached/v1alpha1
	Log:
		Loaded 1 Custom Resources from alm-examples
		Loaded CustomresourceDefinitions: [&CustomResourceDefinition{ObjectMeta:{memcacheds.cac

Describe the solution you'd like

Have a target similar to make manifests for Helm/Ansible.

Maybe we could implement a similar feature which would:

@kensipe kensipe added this to the Backlog milestone Aug 24, 2020
@joelanford
Copy link
Member

IMO, we should rely on upstream support for schema validation specifications. See Helm v3's new support, for example: helm/helm#5081

@kensipe kensipe added kind/feature Categorizes issue or PR as related to a new feature. needs discussion labels Aug 24, 2020
@joelanford
Copy link
Member

@camilamacedo86 camilamacedo86 self-assigned this Aug 28, 2020
@camilamacedo86
Copy link
Contributor Author

I will check it as discussed in the meeting triage.
We might need to push changes in the upstream in order to let us know easily what would be the attributes and types.

@camilamacedo86
Copy link
Contributor Author

camilamacedo86 commented Sep 7, 2020

Hi @joelanford,

I could check this chart schema(helm/helm#5350). See its doc:

values.schema.json # OPTIONAL: A JSON Schema for imposing a structure on the values.yaml file

So, for Helm, we could address it by using this schema. Also, if we would like to parse the YAML file and get the attributes and its types to create a make manifests for Helm/Ansible we could do something such as too:

package main

import (
	"fmt"
	"reflect"
	"sigs.k8s.io/yaml"
)

var file = `---
foo1:
  bar:
    - bar
    - rab
    - plop
foo2: bar
foo3: 1
foo4: true
foo5: 1.4
foo8:
  - bar
  - rab
  - plop
`

func main() {
	var data map[string]interface{}
	yaml.Unmarshal([]byte(file), &data)
	for key, value := range data {
		valueType := reflect.TypeOf(value).Kind().String()
		fmt.Println("Attribute:", key, "Type:", valueType )
	}
}

Result:

Attribute: foo1 Type: map
Attribute: foo2 Type: string
Attribute: foo3 Type: float64
Attribute: foo4 Type: bool
Attribute: foo5 Type: float64
Attribute: foo8 Type: slice

@camilamacedo86
Copy link
Contributor Author

The EP operator-framework/enhancements#25 for OLM also shows requires to parse the chart values to create the manifests.

@camilamacedo86 camilamacedo86 removed their assignment Oct 22, 2020
@joelanford
Copy link
Member

I have a working branch that uses values.schema.json if it is present in a chart.

While developing it, I noticed that Kubernetes API server is fairly strict on what it expects to be present or not present in the CRD validation section when converting from values.schema.json.

This is a UX problem because the CRD generation appears to work correctly, but an error is surfaced much later when one actually attempts to create the CRD in a cluster. IMO that is a bad user experience, and one we should try to avoid.

@camilamacedo86
Copy link
Contributor Author

The end goal of this issue would be we have a makefile target manifests for Ansible/Go. I will update the first comment to clarifies it. It cames in the survey so, I am removing the milestone for we have the chance to discuss it again in the bug triage.

@camilamacedo86 camilamacedo86 changed the title RFE: Feature/lib to update k8s config manifests based on the chart/roles/playbooks data. Make manifest target for ansible/helm. Oct 30, 2020
@camilamacedo86 camilamacedo86 removed this from the Backlog milestone Oct 30, 2020
@joelanford
Copy link
Member

I think this is what I used experimenting with Helm: https://github.com/karuppiah7890/helm-schema-gen

@kensipe kensipe added the help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. label Nov 2, 2020
@joelanford joelanford changed the title Make manifest target for ansible/helm. Enable CRD validation generation for Helm and Ansible Nov 2, 2020
@kensipe kensipe added this to the Backlog milestone Nov 2, 2020
@joelanford
Copy link
Member

If someone cares about this and would like to help us investigate or implement, please reach out to the maintainers to continue the discussion about how this could work.

@openshift-bot
Copy link

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

@openshift-ci-robot openshift-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 1, 2021
@openshift-bot
Copy link

Stale issues rot after 30d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle rotten
/remove-lifecycle stale

@openshift-ci-robot openshift-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Mar 3, 2021
@openshift-bot
Copy link

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

@openshift-ci-robot
Copy link

@openshift-bot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@camilamacedo86
Copy link
Contributor Author

I am re-open this one. I think that would be very helpful if we would be able to move forward with.

@openshift-bot
Copy link

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

@openshift-ci
Copy link

openshift-ci bot commented Jun 13, 2021

@openshift-bot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci openshift-ci bot closed this as completed Jun 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. needs discussion
Projects
None yet
Development

No branches or pull requests

5 participants