Add configs section to App CRD #38
Conversation
|
Hold on, I just realised this repo works differently than I expected and it hosts the structs for the CRDs as well. |
|
All right, did it properly this time and also aligned the recently updated docs for |
| type AppConfiguration struct { | ||
| // Kind of configuration to look up that should be applied to the app when deployed. | ||
| // +kubebuilder:validation:Enum=configMap;secret | ||
| Kind string `json:"kind"` |
There was a problem hiding this comment.
Should there also be a default to configmap here?
There was a problem hiding this comment.
In my opinion explicit is better than implicit. Or do you mean we should allow configmap and configMap as a value as well? I chose configMap for 2 reasons: this is how the RFC calls it and this is how we use it in other places as we. within the same CRD.
There was a problem hiding this comment.
Sorry I meant a kubebuilder default to set this Kind field to configMap by default.
There was a problem hiding this comment.
@MarcelMue @kubasobon I set the kind field to optional and the default for it as configMap. I can see the benefits of it as probably it will be config map most of the time and this saves some repetition I guess.
uvegla
force-pushed
the
implement-configs-list-for-app-cr
branch
2 times, most recently
from
9db9409 to
6c89c12
Compare
These will be used in merging logics and makes sense to keep with the actual configuration.
uvegla
force-pushed
the
implement-configs-list-for-app-cr
branch
from
6c89c12 to
843337a
Compare
Towards: https://github.com/giantswarm/giantswarm/issues/22593
Checklist