Skip to content

Commit a6222b7

Browse files
committed
Document how to CRD. Add helm copier.
- Add how to create CRDs to contributing.md. - Add a script that will copy new CRDs to consul-helm in the format expected. This will reduce manual copying errors.
1 parent 1fb37fa commit a6222b7

File tree

3 files changed

+421
-0
lines changed

3 files changed

+421
-0
lines changed

CONTRIBUTING.md

Lines changed: 324 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,3 +57,327 @@ If the changes in your PR do not conflict with any of the existing code in the p
5757
automatic rebasing when the PR is accepted into the code. However, if there are conflicts (there will be
5858
a warning on the PR that reads "This branch cannot be rebased due to conflicts"), you will need to manually
5959
rebase the branch on master, fixing any conflicts along the way before the code can be merged.
60+
61+
## Creating a new CRD
62+
63+
### The Structs
64+
1. Run the generate command:
65+
```bash
66+
operator-sdk create api --group consul --version v1alpha1 --kind IngressGateway --controller --namespaced=true --make=false --resource=true
67+
```
68+
1. Re-order the file so it looks like:
69+
```go
70+
func init() {
71+
SchemeBuilder.Register(&IngressGateway{}, &IngressGatewayList{})
72+
}
73+
74+
// +kubebuilder:object:root=true
75+
// +kubebuilder:subresource:status
76+
77+
// IngressGateway is the Schema for the ingressgateways API
78+
type IngressGateway struct {
79+
metav1.TypeMeta `json:",inline"`
80+
metav1.ObjectMeta `json:"metadata,omitempty"`
81+
82+
Spec IngressGatewaySpec `json:"spec,omitempty"`
83+
Status IngressGatewayStatus `json:"status,omitempty"`
84+
}
85+
86+
// +kubebuilder:object:root=true
87+
88+
// IngressGatewayList contains a list of IngressGateway
89+
type IngressGatewayList struct {
90+
metav1.TypeMeta `json:",inline"`
91+
metav1.ListMeta `json:"metadata,omitempty"`
92+
Items []IngressGateway `json:"items"`
93+
}
94+
95+
// IngressGatewaySpec defines the desired state of IngressGateway
96+
type IngressGatewaySpec struct {
97+
// INSERT ADDITIONAL SPEC FIELDS - desired state of cluster
98+
// Important: Run "make" to regenerate code after modifying this file
99+
100+
// Foo is an example field of IngressGateway. Edit IngressGateway_types.go to remove/update
101+
Foo string `json:"foo,omitempty"`
102+
}
103+
104+
// IngressGatewayStatus defines the observed state of IngressGateway
105+
type IngressGatewayStatus struct {
106+
// INSERT ADDITIONAL STATUS FIELD - define observed state of cluster
107+
// Important: Run "make" to regenerate code after modifying this file
108+
}
109+
```
110+
1. Add kubebuilder status metadata to the `IngressGateway struct`:
111+
```go
112+
// ServiceRouter is the Schema for the servicerouters API
113+
// +kubebuilder:printcolumn:name="Synced",type="string",JSONPath=".status.conditions[?(@.type==\"Synced\")].status",description="The sync status of the resource with Consul"
114+
// +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp",description="The age of the resource"
115+
type ServiceRouter struct {
116+
```
117+
1. Delete `IngressGatewayStatus` struct. We use a common status struct.
118+
1. Use the `Status` struct instead and embed it:
119+
```diff
120+
// IngressGateway is the Schema for the ingressgateways API
121+
type IngressGateway struct {
122+
metav1.TypeMeta `json:",inline"`
123+
metav1.ObjectMeta `json:"metadata,omitempty"`
124+
125+
Spec IngressGatewaySpec `json:"spec,omitempty"`
126+
- Status IngressGatewayStatus `json:"status,omitempty"`
127+
+ Status `json:"status,omitempty"`
128+
}
129+
```
130+
1. Go to the Consul `api` package for the config entry, e.g. https://github.com/hashicorp/consul/blob/master/api/config_entry_gateways.go
131+
1. Copy the top-level fields over into the `Spec` struct except for
132+
`Kind`, `Name`, `Namespace`, `Meta`, `CreateIndex` and `ModifyIndex`. In this
133+
example, the top-level fields remaining are `TLS` and `Listeners`:
134+
135+
```go
136+
// IngressGatewaySpec defines the desired state of IngressGateway
137+
type IngressGatewaySpec struct {
138+
// TLS holds the TLS configuration for this gateway.
139+
TLS GatewayTLSConfig
140+
// Listeners declares what ports the ingress gateway should listen on, and
141+
// what services to associated to those ports.
142+
Listeners []IngressListener
143+
}
144+
```
145+
1. Copy the structs over that are missing, e.g. `GatewayTLSConfig`, `IngressListener`.
146+
1. Set `json` tags for all fields using camelCase starting with a lowercase letter:
147+
```go
148+
TLS GatewayTLSConfig `json:"tls"`
149+
```
150+
Note that you should use the fields name, e.g. `tls`, not the struct name, e.g. `gatewayTLSConfig`.
151+
Remove any `alias` struct tags.
152+
1. If the fields aren't documented, document them using the Consul docs as a reference.
153+
154+
### Spec Methods
155+
1. Run `make ctrl-generate` to implement the deep copy methods.
156+
1. Implement all the methods for `ConfigEntryResource` in the `_types.go` file. If using goland you can
157+
automatically stub out all the methods by using Code -> Generate -> IngressGateway -> ConfigEntryResource.
158+
1. Use existing implementations of other types to implement the methods. We have to
159+
copy their code because we can't use a common struct that implements the methods
160+
because that messes up the CRD code generation.
161+
162+
You should be able to follow the other "normal" types. The non-normal types
163+
are `ServiceIntention` and `ProxyDefault` because they have special behaviour
164+
around being global or their spec not matching up with Consul's directly.
165+
1. When you get to `ToConsul` and `Validate` you'll need to actually think
166+
about the implementation instead of copy/pasting and doing a simple replace.
167+
1. For `ToConsul`, the pattern we follow is to implement `toConsul()` methods
168+
on each sub-struct. You can see this pattern in the existing types.
169+
1. For `Validate`, we again follow the pattern of implementing the method on
170+
each sub-struct. You'll need to read the Consul documentation to understand
171+
what validation needs to be done.
172+
173+
Things to keep in mind:
174+
1. Re-use the `sliceContains` and `notInSliceMessage` helper methods where applicable.
175+
1. If the invalid field is an entire struct, encode as json (look for `asJSON` for an example).
176+
1. `validateNamespaces` should be a separate method.
177+
1. If the field can have a `nil` pointer, check for that, e.g.
178+
```go
179+
func (in *ServiceRouteHTTPMatchHeader) validate(path *field.Path) *field.Error {
180+
if in == nil {
181+
return nil
182+
}
183+
```
184+
185+
### Spec Tests
186+
1. Create a test file, e.g. `ingressgateway_types_test.go`.
187+
1. Copy the tests for the `ConfigEntryResource` methods from another type and search and replace.
188+
Only the tests for `ToConsul()`, `Validate()` and `MatchesConsul()` need to
189+
be implemented without copying.
190+
1. The test for `MatchesConsul` will look like:
191+
```go
192+
func TestIngressGateway_MatchesConsul(t *testing.T) {
193+
cases := map[string]struct {
194+
Ours IngressGateway
195+
Theirs capi.ConfigEntry
196+
Matches bool
197+
}{
198+
"empty fields matches": {
199+
"all fields set matches": {
200+
"different types does not match": {
201+
202+
}
203+
204+
for name, c := range cases {
205+
t.Run(name, func(t *testing.T) {
206+
require.Equal(t, c.Matches, c.Ours.MatchesConsul(c.Theirs))
207+
})
208+
}
209+
}
210+
```
211+
1. The test for `ToConsul` will re-use the same cases as for `MatchesConsul()`
212+
with the following modifications:
213+
1. The case with `empty field matches` will use the same struct, but the case will be renamed to `empty fields`
214+
1. The case with `all fields set matches` will be renamed to `every field set`
215+
1. All cases will remove the `Namespace` and `CreateIndex`/`ModifyIndex` fields
216+
since the `ToConsul` method won't set those
217+
1. The test for `Validate` should exercise all the validations you wrote.
218+
219+
### Controller
220+
1. Delete the file `controllers/suite_test.go`. We don't write suite tests, just unit tests.
221+
1. Move `controllers/ingressgateway_controller.go` to `controller` directory.
222+
1. Delete the `controllers` directory.
223+
1. Rename `Reconciler` to `Controller`, e.g. `IngressGatewayReconciler` => `IngressGatewayController`
224+
1. Use the existing controller files as a guide and make this file match.
225+
1. Add your controller as a case in the tests in `configentry_controller_test.go`:
226+
1. `TestConfigEntryControllers_createsConfigEntry`
227+
1. `TestConfigEntryControllers_updatesConfigEntry`
228+
1. `TestConfigEntryControllers_deletesConfigEntry`
229+
1. `TestConfigEntryControllers_errorUpdatesSyncStatus`
230+
1. `TestConfigEntryControllers_setsSyncedToTrue`
231+
1. `TestConfigEntryControllers_doesNotCreateUnownedConfigEntry`
232+
1. `TestConfigEntryControllers_doesNotDeleteUnownedConfig`
233+
1. Note: we don't add tests to `configentry_controller_ent_test.go` because we decided
234+
it's too much duplication and the controllers are already properly exercised in the oss tests.
235+
236+
### Webhook
237+
1. Copy an existing webhook to `api/v1alpha/ingressgateway_webhook.go`
238+
1. Replace the names
239+
1. Ensure you've correctly replaced the names in the kubebuilder annotation, ensure the plurality is correct
240+
```go
241+
// +kubebuilder:webhook:verbs=create;update,path=/mutate-v1alpha1-ingressgateway,mutating=true,failurePolicy=fail,groups=consul.hashicorp.com,resources=ingressgateways,versions=v1alpha1,name=mutate-ingressgateway.consul.hashicorp.com,webhookVersions=v1beta1,sideEffects=None
242+
```
243+
244+
### Update command.go
245+
1. Add your resource name to `api/common/common.go`:
246+
```go
247+
const (
248+
...
249+
IngressGateway string = "ingressgateway"
250+
```
251+
1. Update `subcommand/controller/command.go` and add your controller:
252+
```go
253+
if err = (&controller.IngressGatewayController{
254+
ConfigEntryController: configEntryReconciler,
255+
Client: mgr.GetClient(),
256+
Log: ctrl.Log.WithName("controller").WithName(common.IngressGateway),
257+
Scheme: mgr.GetScheme(),
258+
}).SetupWithManager(mgr); err != nil {
259+
setupLog.Error(err, "unable to create controller", "controller", common.IngressGateway)
260+
return 1
261+
}
262+
```
263+
1. Update `subcommand/controller/command.go` and add your webhook (the path should match the kubebuilder annotation):
264+
```go
265+
mgr.GetWebhookServer().Register("/mutate-v1alpha1-ingressgateway",
266+
&webhook.Admission{Handler: &v1alpha1.IngressGatewayWebhook{
267+
Client: mgr.GetClient(),
268+
ConsulClient: consulClient,
269+
Logger: ctrl.Log.WithName("webhooks").WithName(common.IngressGateway),
270+
EnableConsulNamespaces: c.flagEnableNamespaces,
271+
EnableNSMirroring: c.flagEnableNSMirroring,
272+
}})
273+
```
274+
275+
### Generating YAML
276+
1. Run `make ctrl-manifests` to generate the CRD and webhook YAML.
277+
1. Uncomment your CRD in `config/crd/kustomization` under `patchesStrategicMerge:`
278+
1. Update the sample, e.g. `config/samples/consul_v1alpha1_ingressgateway.yaml` to a valid resource
279+
that can be used for testing:
280+
```yaml
281+
apiVersion: consul.hashicorp.com/v1alpha1
282+
kind: IngressGateway
283+
metadata:
284+
name: ingressgateway-sample
285+
spec:
286+
tls:
287+
enabled: false
288+
listeners:
289+
- port: 8080
290+
protocol: "tcp"
291+
services:
292+
- name: "foo"
293+
```
294+
295+
### Updating consul-helm
296+
1. To copy the CRD YAML into the consul-helm repo, run `make ctrl-crd-copy helm=<path-to-consul-helm>`
297+
where `<path-to-consul-helm>` is the relative or absolute path to your consul-helm repository.
298+
This will copy your CRD into consul-helm with the required formatting.
299+
1. In consul-helm, update `templates/controller-mutatingwebhookconfiguration` with the webhook for this resource
300+
using the updated `config/webhook/manifests.v1beta1.yaml` and replacing `clientConfig.service.name/namespace`
301+
with the templated strings shown below to match the other webhooks.:
302+
```yaml
303+
- clientConfig:
304+
service:
305+
name: {{ template "consul.fullname" . }}-controller-webhook
306+
namespace: {{ .Release.Namespace }}
307+
path: /mutate-v1alpha1-ingressgateway
308+
failurePolicy: Fail
309+
admissionReviewVersions:
310+
- "v1beta1"
311+
- "v1"
312+
name: mutate-ingressgateway.consul.hashicorp.com
313+
rules:
314+
- apiGroups:
315+
- consul.hashicorp.com
316+
apiVersions:
317+
- v1alpha1
318+
operations:
319+
- CREATE
320+
- UPDATE
321+
resources:
322+
- ingressgateways
323+
sideEffects: None
324+
```
325+
1. Update `templates/controller-clusterrole.yaml` to allow the controller to
326+
manage your resource type.
327+
328+
### Test
329+
1. Build a Docker image for consul-k8s via `make dev-docker` and tagging your image appropriately.
330+
1. Install using the updated Helm repository, with a values like:
331+
```yaml
332+
global:
333+
imageK8S: ghcr.io/lkysow/consul-k8s-dev:nov26
334+
name: consul
335+
server:
336+
replicas: 1
337+
bootstrapExpect: 1
338+
controller:
339+
enabled: true
340+
```
341+
1. `kubectl apply` your sample CRD.
342+
1. Check its synced status:
343+
```bash
344+
kubectl get ingressgateway
345+
NAME SYNCED AGE
346+
ingressgateway-sample True 8s
347+
```
348+
1. Make a call to consul to confirm it was created as expected:
349+
```bash
350+
kubectl exec consul-server-0 -- consul config read -name ingressgateway-sample -kind ingress-gateway
351+
{
352+
"Kind": "ingress-gateway",
353+
"Name": "ingressgateway-sample",
354+
"TLS": {
355+
"Enabled": false
356+
},
357+
"Listeners": [
358+
{
359+
"Port": 8080,
360+
"Protocol": "tcp",
361+
"Services": [
362+
{
363+
"Name": "foo",
364+
"Hosts": null
365+
}
366+
]
367+
}
368+
],
369+
"Meta": {
370+
"consul.hashicorp.com/source-datacenter": "dc1",
371+
"external-source": "kubernetes"
372+
},
373+
"CreateIndex": 57,
374+
"ModifyIndex": 57
375+
}
376+
```
377+
378+
### Update consul-helm Acceptance Tests
379+
1. Add a test resource to `test/acceptance/tests/fixtures/crds/ingressgateway.yaml`. Ideally it requires
380+
no other resources. For example, I used a `tcp` service so it didn't require a `ServiceDefaults`
381+
resource to set its protocol to something else.
382+
1. Update `test/acceptance/tests/controller/controller_test.go` and `test/acceptance/tests/controller/controller_namespaces_test.go`.
383+
1. Test locally, then submit a PR that uses your Docker image as `global.imageK8S`.

Makefile

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,11 @@ ctrl-manifests: controller-gen
148148
ctrl-generate: controller-gen
149149
$(CONTROLLER_GEN) object:headerFile="build-support/controller/boilerplate.go.txt" paths="./..."
150150

151+
# Copy CRD YAML to consul-helm.
152+
# Usage: make ctrl-crd-copy helm=<path-to-consul-helm-repo>
153+
ctrl-crd-copy:
154+
@cd hack/crds-to-consul-helm; go run ./... $(helm)
155+
151156
# find or download controller-gen
152157
# download controller-gen if necessary
153158
controller-gen:

0 commit comments

Comments
 (0)