Skip to content
This repository was archived by the owner on Oct 30, 2024. It is now read-only.

Improve CRD generation#410

Merged
xh3b4sd merged 81 commits intomasterfrom
improve-generation
Apr 16, 2020
Merged

Improve CRD generation#410
xh3b4sd merged 81 commits intomasterfrom
improve-generation

Conversation

@tfussell
Copy link
Contributor

@tfussell tfussell commented Apr 14, 2020

As a follow-up from #409, this PR allows CRDs to be loaded in go code as structs from the generated YAMLs. Most of the interesting changes are in the scripts directory.

Checklist

  • Consider SIG UX feedback.
  • Update changelog in CHANGELOG.md.

Comment on lines -36 to -100
openAPIV3Schema:
description: |
The CertConfig resource is used in a Giant Swarm installation to ensure TLS communication between
a component (e. g. prometheus) and the tenant cluster nodes. It is reconciled by cert-operator.
For each CertConfig resource, cert-operator ensures the existence of an X.509 certificate as
defined in RFC 5280.
properties:
spec:
type: object
properties:
cert:
description: |
Defines an X.509 certificate to be ensured by cert-operator.
type: object
properties:
allowBareDomains:
description: |
Specifies if clients can request certificates matching the value of the actual
domains themselves.
type: bool
altNames:
description: |
Specifies host names to set in the certificate as Subject Alternative Names.
type: array
items:
type: string
clusterComponent:
description: |
Name of the component this certificate is for.
type: string
clusterID:
description: |
Unique identifier of the tenant cluster this certificate is for.
type: string
commonName:
description: |
The value of the Common Name (CN) attribute of the certificate.
disableRegeneration:
description: |
Disable automatic certificate rotation before expiry.
type: bool
ipSans:
description: |
Specifies requested IP Subject Alternative Names to be set in the
certificate.
type: array
items:
type: string
organizations:
description: |
Organizations to set in the Organizations (O) attribute of the
certificate.
type: array
items:
type: string
ttl:
description: |
Expiry duration after creation. The value must consist of a number
combined with a unit, without blanks, to be parsed by the Go
[time.ParseDuration](https://golang.org/pkg/time/#ParseDuration) function.
type: string
versionBundle:
description: |
No longer used
type: object
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still don't see where these descriptions are going. The according go fields don't have any comments, so the only way to recover this information will be to dig in the git history of the repo if we remove this now. (This CRD is only one example out of several affected.)

tfussell and others added 3 commits April 16, 2020 08:57
* master:
  use architect-orb v0.8.8 (#408)
  Replace DeepCopyTime with metav1.Time (#407)
  Add missing releases to changelog (#411)
  Document G8sControlPlane CRD (#405)
  Add Chart CR (#406)
* generate-crds:
  specify number type for kvmconfig quantity fields
@kopiczko
Copy link
Contributor

I recently added the note about considering UX feedback and I am not so sure anymore this was a good move.

I like that. But surely we shouldn't write books in those templates.

@xh3b4sd
Copy link
Contributor

xh3b4sd commented Apr 16, 2020

We took this over. After testing and gaining confidence we merge this to master now to get a workable tag in other projects. Thanks a lot to everyone especially @tfussell who did some invaluable contributions for the sake of the whole team. It was a bit bumpy but I hope we are all kind of cool with it. We rollin'.

@xh3b4sd xh3b4sd changed the base branch from generate-crds to master April 16, 2020 19:33
@xh3b4sd
Copy link
Contributor

xh3b4sd commented Apr 16, 2020

@marians there are some descriptions missing but I will bring them back next week.

@xh3b4sd xh3b4sd merged commit dab39bf into master Apr 16, 2020
@xh3b4sd xh3b4sd deleted the improve-generation branch April 16, 2020 19:34
@marians
Copy link
Member

marians commented Apr 17, 2020

I'm wondering why there are still two CRD YAML files in docs/crd:

image

Why have those not moved to config/bases/crd?

@marians
Copy link
Member

marians commented Apr 17, 2020

When we work on bringing back descriptions, and we use Go comments for them, can we agree that we don't have to repeat the name of the property in the comment? We already have this here with the LastTransitionTime property:

image

It doesn't seem to trigger any lint/fmt errors in my tooling (Go 1.14).

singular: release
preserveUnknownFields: false
scope: Cluster
scope: Namespaced
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did this change from cluster to namespace scoped? Just curious..

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uncertain, but I think @tfussell knows more about it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to add // +kubebuilder:resource:scope=Cluster to CRs to get the back to parity with our old manually defined CRDs. I can do some if it, but the work will hopefully be divided into teams responsible for each CRD.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants