Add APIKeysKey go types generated from proto #1872
Conversation
justinsb
commented
May 22, 2024
justinsb
commented
- chore: reflow descriptions when generating CRDs
- chore: add APIKeysKey go types generated from proto
- autogen: generate direct-actuation CRDs
justinsb
force-pushed
the
generate_apikeys
branch
2 times, most recently
from
5cca035
to
264b56a
Compare
| // DA39A3EE5E6B4B0D3255BFEF95601890AFD80709. | ||
| // Output format is the latter. | ||
| // +required | ||
| Sha1Fingerprint *string `json:"sha1Fingerprint,omitempty"` |
There was a problem hiding this comment.
Not sure if that's feasible. I hope that we can detect the "+required" and add a kubebuilder/kcc validator.
There was a problem hiding this comment.
So +required works now, after kubernetes-sigs/controller-tools#944
We don't yet validate the input format (though I would expect GCP to reject it if invalid, so the user will still get a reasonable error, we shouldn't fail silently)
Because the input and output formats are potentially different, we'll need to be careful here, we can't just do a naive diff to determine if we need to re-reconcile.
There was a problem hiding this comment.
So
+requiredworks now, after kubernetes-sigs/controller-tools#944
Ah! This is so fresh and good news!
We don't yet validate the input format (though I would expect GCP to reject it if invalid, so the user will still get a reasonable error, we shouldn't fail silently)
Agree. I think I want to have basic and general validation on CRD, and have more server specific/usage related validation from the GCP server.
Because the input and output formats are potentially different, we'll need to be careful here, we can't just do a naive diff to determine if we need to re-reconcile.
Yes. This makes a lot of sense.
| @@ -38,6 +38,7 @@ go run ./scripts/crd-tools set-field spec.preserveUnknownFields=false --dir apis | |||
| go run ./scripts/crd-tools delete-field status --dir apis/config/crd/ | |||
| go run ./scripts/crd-tools set-annotation cnrm.cloud.google.com/version=0.0.0-dev --dir apis/config/crd/ | |||
| go run ./scripts/crd-tools delete-annotation controller-gen.kubebuilder.io/version --dir apis/config/crd/ | |||
| go run ./scripts/crd-tools reflow-descriptions --dir apis/config/crd/ | |||
There was a problem hiding this comment.
We don't have apis/config/crd dir yet. Is this for future structure?
There was a problem hiding this comment.
So actually this script generates that directory. We don't check it in currently, and it does get "left behind" if you run it locally. I'm not sure if we should delete it in the script (or generate it under .build). WDYT?
There was a problem hiding this comment.
Oh I see what this script does. (I was more thinking about keeping config/crds as the single CRD dir)
I"ll vote for .build.
| return s | ||
| } | ||
|
|
||
| func (v *visitor) visitDescription(s string) string { |
There was a problem hiding this comment.
Yeah, we're going to need a package for visitors/object walkers at some point .. it keeps coming up :-)
justinsb
force-pushed
the
generate_apikeys
branch
from
264b56a
to
c2a2298
Compare
This avoids whitespace changes which make the diff harder to review.
We have a test that detected this field is being removed (it's an alpha resource, so not a problem). Remove the mapping to keep the test happy.
justinsb
force-pushed
the
generate_apikeys
branch
from
c2a2298
to
d0df79d
Compare
|
/lgtm Thank you for this PR. very cool! |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: yuwenma The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
google-oss-prow
bot
merged commit 5ee1539
into
GoogleCloudPlatform:master
