From 85d6445d2c1110e1b98c41a438d0cddac34e03a0 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Tue, 3 Dec 2024 11:58:41 -0500 Subject: [PATCH 1/6] docs(readme): point user to OperatorHub for installation, move development steps to end (#980) --- README.md | 63 ++++++++++++++++++++++++++++++------------------------- 1 file changed, 35 insertions(+), 28 deletions(-) diff --git a/README.md b/README.md index 9ef48c14..d1dfbe7f 100644 --- a/README.md +++ b/README.md @@ -46,36 +46,14 @@ you may use the Cryostat web UI to define a Custom Target with the connection UR This is a special value which tells Cryostat's JVM that it should connect to itself directly, without the need to expose a JMX port over the network. -## BUILDING - -### Requirements -- `go` v1.21+ -- [`operator-sdk`](https://github.com/operator-framework/operator-sdk) v1.31.0 -- `podman` or `docker` -- [`jq`](https://stedolan.github.io/jq/) v1.6+ -- [`yq`](https://github.com/mikefarah/yq/) v4.35+ -- `ginkgo` (Optional) - -### Instructions -`make generate manifests manager` will trigger code/YAML generation and compile -the operator controller manager, along with running some code quality checks. - -`make oci-build` will build an OCI image from the generated YAML and compiled -binary to the local registry, tagged as -`quay.io/crystatio/cryostat-operator`. This tag can be overridden by -setting the environment variables `IMAGE_NAMESPACE` and `OPERATOR_NAME`. -`IMAGE_VERSION` can also be set to override the tagged version. - -`make bundle` will create an OLM bundle. This will generate a CSV, CRDs and -other manifests, and other required configurations for an OLM bundle versioned -with version `$IMAGE_VERSION` in the `bundle/` directory. `make bundle-build` -will create an OCI image of this bundle, which can then be pushed to an image -repository such as `quay.io`. +## INSTALLATION -`make catalog-build` will build an OCI image of the operator catalog (i.e. index) -with version `$IMAGE_VERSION` that includes the bundle image of the same version. +### OperatorHub -## SETUP / DEPLOYMENT +The operator's primary installation method is via [OperatorHub](https://operatorhub.io/). +A more detailed installation guide is available [here](https://cryostat.io/get-started/#install-via-operatorhub). +Installation of the Operator without OLM/OperatorHub is intended for development purposes and is +not a supported release configuration. ### Bundle Deployment @@ -191,6 +169,35 @@ $ oc get -o jsonpath='{.data.token}' secret cryostat-operator-service-account-to eyJhbGciOiJSUzI1NiIsImtpZCI6IkhYZC13eDdGVGwyQzdGNVpZVndScEZ2VmRxWTlzbnBUUG9HRkJpejJkV3cifQ.eyJpc3MiOiJrdWJlcm5ldGVzL3NlcnZpY2VhY2NvdW50Iiwia3ViZXJuZXRlcy5pby9zZXJ2aWNlYWNjb3VudC9uYW1lc3BhY2UiOiJkZWZhdWx0Iiwia3ViZXJuZXRlcy5pby9zZXJ2aWNlYWNjb3VudC9zZWNyZXQubmFtZSI6ImNyeW9zdGF0LW9wZXJhdG9yLXNlcnZpY2UtYWNjb3VudC10b2tlbi03dHQ3bCIsImt1YmVybmV0ZXMuaW8vc2VydmljZWFjY291bnQvc2VydmljZS1hY2NvdW50Lm5hbWUiOiJjcnlvc3RhdC1vcGVyYXRvci1zZXJ2aWNlLWFjY291bnQiLCJrdWJlcm5ldGVzLmlvL3NlcnZpY2VhY2NvdW50L3NlcnZpY2UtYWNjb3VudC51aWQiOiI2OTJhYThjNy0wODFlLTRhNTEtOTM1NS1iZTNlYWE4ZjlmYTYiLCJzdWIiOiJzeXN0ZW06c2VydmljZWFjY291bnQ6ZGVmYXVsdDpjcnlvc3RhdC1vcGVyYXRvci1zZXJ2aWNlLWFjY291bnQifQ.M7C1V0bN3aILBflO7TTOTikw7wLGRJ79-OkCDQIZbu71QLdX05jyCxxtlH32lr8jz6HwxfXXweh3ifG_2lbe7_TbM8jxmBoMdLuc4Q_akpmA-GQuDPrRxfHGJApYGQ6CVug3KHSrQwj2M4QrSUz7FoeQGaOH9BnWj1TrHGmOZUPJ6u7JSu2OwoLBda6rF-M4Bl72DmkyMAzikreRgPEk4D7gTCY0yNvsQDuUAwpFwmEukRC2WyTAVTpKPgThZUk-UJ-dXufbhAcqIRt6jeCQ19_Bo0zXc_ELgQydxuTack1ndT3HwRmwwNuZDFv-G3Y0YdjfRh00DqEvSn9ynZzwueDCJUxlHdznytfUWk9PA712JENpFC7b-zSHnjymIcFeUd8s_Zq_-JKrDIPnH0oZDRO_MUpKEC7Jz_8SeFJHLLGfBZt_aP4VwQHEUThiFQPwrfbd8tppUG2TKcekPScKcauy-BCI52odBzapP6meilMQVrmRtu7i30L05vgQiST_OsmSP8CuKW13a-leCCtN_aNQGqlWvLhP81H95ui-PvMzwMIDlfDZ03ycuYg4R4eUG3nUq7-42wrSdFLo8gm9wsl7y1ZRMQwHR1DCVBbHYS0iFOcmwto2Ejlrgvn3Cs0pDS7pDVoFkH2FsTopEw3jXtnkMs15mSmBnHz-UjF-l08 ``` +## BUILDING + +### Requirements +- `go` v1.21+ +- [`operator-sdk`](https://github.com/operator-framework/operator-sdk) v1.31.0 +- `podman` or `docker` +- [`jq`](https://stedolan.github.io/jq/) v1.6+ +- [`yq`](https://github.com/mikefarah/yq/) v4.35+ +- `ginkgo` (Optional) + +### Instructions +`make generate manifests manager` will trigger code/YAML generation and compile +the operator controller manager, along with running some code quality checks. + +`make oci-build` will build an OCI image from the generated YAML and compiled +binary to the local registry, tagged as +`quay.io/crystatio/cryostat-operator`. This tag can be overridden by +setting the environment variables `IMAGE_NAMESPACE` and `OPERATOR_NAME`. +`IMAGE_VERSION` can also be set to override the tagged version. + +`make bundle` will create an OLM bundle. This will generate a CSV, CRDs and +other manifests, and other required configurations for an OLM bundle versioned +with version `$IMAGE_VERSION` in the `bundle/` directory. `make bundle-build` +will create an OCI image of this bundle, which can then be pushed to an image +repository such as `quay.io`. + +`make catalog-build` will build an OCI image of the operator catalog (i.e. index) +with version `$IMAGE_VERSION` that includes the bundle image of the same version. + ## DEVELOPMENT An invocation like From 75b7a81fb1fe6041204669f683d5b789ec01b64c Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Tue, 17 Dec 2024 11:41:52 -0500 Subject: [PATCH 2/6] feat(service): add part-of, instance selector labels and appProtocol (#974) --- internal/controllers/services.go | 14 +++++--- internal/test/resources.go | 62 ++++++++++++++++++++++---------- 2 files changed, 54 insertions(+), 22 deletions(-) diff --git a/internal/controllers/services.go b/internal/controllers/services.go index 58ec16f5..5be4c1ac 100644 --- a/internal/controllers/services.go +++ b/internal/controllers/services.go @@ -47,11 +47,13 @@ func (r *Reconciler) reconcileCoreService(ctx context.Context, cr *model.Cryosta "app": cr.Name, "component": "cryostat", } + appProtocol := "http" svc.Spec.Ports = []corev1.ServicePort{ { - Name: "http", - Port: *config.HTTPPort, - TargetPort: intstr.IntOrString{IntVal: constants.AuthProxyHttpContainerPort}, + Name: "http", + Port: *config.HTTPPort, + TargetPort: intstr.IntOrString{IntVal: constants.AuthProxyHttpContainerPort}, + AppProtocol: &appProtocol, }, } return nil @@ -188,7 +190,7 @@ func configureAgentService(cr *model.CryostatInstance) *operatorv1beta2.AgentSer } // Apply common service defaults - configureService(&config.ServiceConfig, cr.Name, "cryostat") + configureService(&config.ServiceConfig, cr.Name, "cryostat-agent-gateway") // Apply default HTTP port if not provided if config.HTTPPort == nil { @@ -214,6 +216,10 @@ func configureService(config *operatorv1beta2.ServiceConfig, appLabel string, co // Add required labels, overriding any user-specified labels with the same keys config.Labels["app"] = appLabel config.Labels["component"] = componentLabel + config.Labels["app.kubernetes.io/name"] = "cryostat" + config.Labels["app.kubernetes.io/instance"] = appLabel + config.Labels["app.kubernetes.io/component"] = componentLabel + config.Labels["app.kubernetes.io/part-of"] = "cryostat" } func (r *Reconciler) createOrUpdateService(ctx context.Context, svc *corev1.Service, owner metav1.Object, diff --git a/internal/test/resources.go b/internal/test/resources.go index 0e804e80..bc74bc9f 100644 --- a/internal/test/resources.go +++ b/internal/test/resources.go @@ -760,13 +760,18 @@ func (r *TestResources) NewCryostatWithAdditionalMetadata() *model.CryostatInsta } func (r *TestResources) NewCryostatService() *corev1.Service { + appProtocol := "http" return &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ Name: r.Name, Namespace: r.Namespace, Labels: map[string]string{ - "app": r.Name, - "component": "cryostat", + "app": r.Name, + "component": "cryostat", + "app.kubernetes.io/name": "cryostat", + "app.kubernetes.io/instance": r.Name, + "app.kubernetes.io/component": "cryostat", + "app.kubernetes.io/part-of": "cryostat", }, }, Spec: corev1.ServiceSpec{ @@ -777,9 +782,10 @@ func (r *TestResources) NewCryostatService() *corev1.Service { }, Ports: []corev1.ServicePort{ { - Name: "http", - Port: 4180, - TargetPort: intstr.FromInt(4180), + Name: "http", + Port: 4180, + TargetPort: intstr.FromInt(4180), + AppProtocol: &appProtocol, }, }, }, @@ -792,8 +798,12 @@ func (r *TestResources) NewReportsService() *corev1.Service { Name: r.Name + "-reports", Namespace: r.Namespace, Labels: map[string]string{ - "app": r.Name, - "component": "reports", + "app": r.Name, + "component": "reports", + "app.kubernetes.io/name": "cryostat", + "app.kubernetes.io/instance": r.Name, + "app.kubernetes.io/component": "reports", + "app.kubernetes.io/part-of": "cryostat", }, }, Spec: corev1.ServiceSpec{ @@ -819,8 +829,12 @@ func (r *TestResources) NewAgentProxyService() *corev1.Service { Name: r.Name + "-agent", Namespace: r.Namespace, Labels: map[string]string{ - "app": r.Name, - "component": "cryostat", + "app": r.Name, + "component": "cryostat-agent-gateway", + "app.kubernetes.io/name": "cryostat", + "app.kubernetes.io/instance": r.Name, + "app.kubernetes.io/component": "cryostat-agent-gateway", + "app.kubernetes.io/part-of": "cryostat", }, }, Spec: corev1.ServiceSpec{ @@ -848,9 +862,13 @@ func (r *TestResources) NewCustomizedCoreService() *corev1.Service { "my/custom": "annotation", } svc.Labels = map[string]string{ - "app": r.Name, - "component": "cryostat", - "my": "label", + "app": r.Name, + "component": "cryostat", + "my": "label", + "app.kubernetes.io/name": "cryostat", + "app.kubernetes.io/instance": r.Name, + "app.kubernetes.io/component": "cryostat", + "app.kubernetes.io/part-of": "cryostat", } return svc } @@ -863,9 +881,13 @@ func (r *TestResources) NewCustomizedReportsService() *corev1.Service { "my/custom": "annotation", } svc.Labels = map[string]string{ - "app": r.Name, - "component": "reports", - "my": "label", + "app": r.Name, + "component": "reports", + "my": "label", + "app.kubernetes.io/name": "cryostat", + "app.kubernetes.io/instance": r.Name, + "app.kubernetes.io/component": "reports", + "app.kubernetes.io/part-of": "cryostat", } return svc } @@ -878,9 +900,13 @@ func (r *TestResources) NewCustomizedAgentService() *corev1.Service { "my/custom": "annotation", } svc.Labels = map[string]string{ - "app": r.Name, - "component": "cryostat", - "my": "label", + "app": r.Name, + "component": "cryostat-agent-gateway", + "my": "label", + "app.kubernetes.io/name": "cryostat", + "app.kubernetes.io/instance": r.Name, + "app.kubernetes.io/component": "cryostat-agent-gateway", + "app.kubernetes.io/part-of": "cryostat", } return svc } From a543918e84dd0c2da8fadb79633048aa5fa92f9c Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Tue, 17 Dec 2024 16:07:13 -0500 Subject: [PATCH 3/6] test(scorecard): adjust scorecard report generation expectation (#985) --- internal/test/scorecard/clients.go | 8 +++----- internal/test/scorecard/tests.go | 4 ++-- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/internal/test/scorecard/clients.go b/internal/test/scorecard/clients.go index 7fa5f83d..fd67351f 100644 --- a/internal/test/scorecard/clients.go +++ b/internal/test/scorecard/clients.go @@ -422,7 +422,7 @@ func (client *RecordingClient) Delete(ctx context.Context, target *Target, recor return nil } -func (client *RecordingClient) GenerateReport(ctx context.Context, target *Target, recording *Recording) (map[string]interface{}, error) { +func (client *RecordingClient) RequestReportGeneration(ctx context.Context, target *Target, recording *Recording) (*string, error) { if len(recording.ReportURL) < 1 { return nil, fmt.Errorf("report URL is not available") } @@ -430,7 +430,6 @@ func (client *RecordingClient) GenerateReport(ctx context.Context, target *Targe reportURL := client.Base.JoinPath(recording.ReportURL) header := make(http.Header) - header.Add("Accept", "application/json") resp, err := SendRequest(ctx, client.Client, http.MethodGet, reportURL.String(), nil, header) if err != nil { @@ -442,13 +441,12 @@ func (client *RecordingClient) GenerateReport(ctx context.Context, target *Targe return nil, fmt.Errorf("API request failed with status code: %d, response body: %s, and headers:\n%s", resp.StatusCode, ReadError(resp), ReadHeader(resp)) } - report := make(map[string]interface{}, 0) - err = ReadJSON(resp, &report) + report, err := ReadString(resp) if err != nil { return nil, fmt.Errorf("failed to read response body: %s", err.Error()) } - return report, nil + return &report, nil } func (client *RecordingClient) ListArchives(ctx context.Context, target *Target) ([]Archive, error) { diff --git a/internal/test/scorecard/tests.go b/internal/test/scorecard/tests.go index 55b90f89..bd287186 100644 --- a/internal/test/scorecard/tests.go +++ b/internal/test/scorecard/tests.go @@ -226,11 +226,11 @@ func CryostatRecordingTest(bundle *apimanifests.Bundle, namespace string, openSh } r.Log += fmt.Sprintf("current list of archives: %+v\n", archives) - report, err := apiClient.Recordings().GenerateReport(context.Background(), target, rec) + reportJobId, err := apiClient.Recordings().RequestReportGeneration(context.Background(), target, rec) if err != nil { return r.fail(fmt.Sprintf("failed to generate report for the recording: %s", err.Error())) } - r.Log += fmt.Sprintf("generated report for the recording %s: %+v\n", rec.Name, report) + r.Log += fmt.Sprintf("report generation job ID for the recording %s: %+v\n", rec.Name, *reportJobId) // Stop the recording err = apiClient.Recordings().Stop(context.Background(), target, rec.Id) From 9ea119d1b9d45689aa2e891a8e0aedc304f2473f Mon Sep 17 00:00:00 2001 From: Elliott Baron Date: Wed, 18 Dec 2024 12:31:29 -0500 Subject: [PATCH 4/6] test(webhook): add test for Cryostat defaulter webhook (#988) --- internal/webhooks/cryostat_webhook.go | 6 +- internal/webhooks/defaulter_test.go | 117 ++++++++++++++++++++++++++ internal/webhooks/validator.go | 20 ++--- internal/webhooks/validator_test.go | 12 +-- 4 files changed, 133 insertions(+), 22 deletions(-) create mode 100644 internal/webhooks/defaulter_test.go diff --git a/internal/webhooks/cryostat_webhook.go b/internal/webhooks/cryostat_webhook.go index 64da3fa4..92ecefbb 100644 --- a/internal/webhooks/cryostat_webhook.go +++ b/internal/webhooks/cryostat_webhook.go @@ -33,9 +33,9 @@ var cryostatlog = logf.Log.WithName("cryostat-resource") func SetupWebhookWithManager(mgr ctrl.Manager, apiType runtime.Object) error { return ctrl.NewWebhookManagedBy(mgr). For(apiType). - WithValidator(&CryostatValidator{ - Client: mgr.GetClient(), - Log: &cryostatlog, + WithValidator(&cryostatValidator{ + client: mgr.GetClient(), + log: &cryostatlog, }). WithDefaulter(&cryostatDefaulter{ log: &cryostatlog, diff --git a/internal/webhooks/defaulter_test.go b/internal/webhooks/defaulter_test.go new file mode 100644 index 00000000..19812f15 --- /dev/null +++ b/internal/webhooks/defaulter_test.go @@ -0,0 +1,117 @@ +// Copyright The Cryostat Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package webhooks_test + +import ( + "context" + "strconv" + + operatorv1beta2 "github.com/cryostatio/cryostat-operator/api/v1beta2" + "github.com/cryostatio/cryostat-operator/internal/controllers/model" + "github.com/cryostatio/cryostat-operator/internal/test" + webhooktests "github.com/cryostatio/cryostat-operator/internal/webhooks/test" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "k8s.io/apimachinery/pkg/types" + ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" + logf "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/log/zap" +) + +type defaulterTestInput struct { + client ctrlclient.Client + objs []ctrlclient.Object + *webhooktests.WebhookTestResources +} + +var _ = Describe("CryostatDefaulter", func() { + var t *defaulterTestInput + var otherNS string + count := 0 + + namespaceWithSuffix := func(name string) string { + return name + "-defaulter-" + strconv.Itoa(count) + } + + BeforeEach(func() { + ns := namespaceWithSuffix("test") + otherNS = namespaceWithSuffix("other") + t = &defaulterTestInput{ + WebhookTestResources: &webhooktests.WebhookTestResources{ + TestResources: &test.TestResources{ + Name: "cryostat", + Namespace: ns, + }, + }, + } + t.objs = []ctrlclient.Object{ + t.NewNamespace(), t.NewOtherNamespace(otherNS), + } + }) + + JustBeforeEach(func() { + logger := zap.New() + logf.SetLogger(logger) + + t.client = k8sClient + for _, obj := range t.objs { + err := t.client.Create(ctx, obj) + Expect(err).ToNot(HaveOccurred()) + } + }) + + JustAfterEach(func() { + for _, obj := range t.objs { + err := ctrlclient.IgnoreNotFound(t.client.Delete(ctx, obj)) + Expect(err).ToNot(HaveOccurred()) + } + }) + + AfterEach(func() { + count++ + }) + + Context("without target namespace", func() { + BeforeEach(func() { + t.objs = append(t.objs, t.NewCryostat().Object) + }) + + It("should set default target namespace", func() { + result := t.getCryostatInstance() + Expect(result.TargetNamespaces).To(ConsistOf(t.Namespace)) + }) + }) + + Context("with target namespace", func() { + BeforeEach(func() { + t.TargetNamespaces = []string{otherNS} + t.objs = append(t.objs, t.NewCryostat().Object) + }) + + It("should do nothing", func() { + result := t.getCryostatInstance() + Expect(result.TargetNamespaces).To(ConsistOf(otherNS)) + }) + }) + +}) + +func (t *defaulterTestInput) getCryostatInstance() *model.CryostatInstance { + cr := &operatorv1beta2.Cryostat{} + err := t.client.Get(context.Background(), types.NamespacedName{Name: t.Name, Namespace: t.Namespace}, cr) + Expect(err).ToNot(HaveOccurred()) + return t.ConvertNamespacedToModel(cr) +} diff --git a/internal/webhooks/validator.go b/internal/webhooks/validator.go index 5b66d548..8b5a9e3a 100644 --- a/internal/webhooks/validator.go +++ b/internal/webhooks/validator.go @@ -27,25 +27,25 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook/admission" ) -type CryostatValidator struct { - Client client.Client - Log *logr.Logger +type cryostatValidator struct { + client client.Client + log *logr.Logger } -var _ admission.CustomValidator = &CryostatValidator{} +var _ admission.CustomValidator = &cryostatValidator{} // ValidateCreate validates a Create operation on a Cryostat -func (r *CryostatValidator) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) { +func (r *cryostatValidator) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) { return r.validate(ctx, obj, "create") } // ValidateCreate validates an Update operation on a Cryostat -func (r *CryostatValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) { +func (r *cryostatValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) { return r.validate(ctx, newObj, "update") } // ValidateCreate validates a Delete operation on a Cryostat -func (r *CryostatValidator) ValidateDelete(ctx context.Context, obj runtime.Object) (admission.Warnings, error) { +func (r *cryostatValidator) ValidateDelete(ctx context.Context, obj runtime.Object) (admission.Warnings, error) { // Nothing to validate on deletion return nil, nil } @@ -68,12 +68,12 @@ func (e *ErrNotPermitted) Error() string { var _ error = &ErrNotPermitted{} -func (r *CryostatValidator) validate(ctx context.Context, obj runtime.Object, op string) (admission.Warnings, error) { +func (r *cryostatValidator) validate(ctx context.Context, obj runtime.Object, op string) (admission.Warnings, error) { cr, ok := obj.(*operatorv1beta2.Cryostat) if !ok { return nil, fmt.Errorf("expected a Cryostat, but received a %T", obj) } - r.Log.Info(fmt.Sprintf("validate %s", op), "name", cr.Name, "namespace", cr.Namespace) + r.log.Info(fmt.Sprintf("validate %s", op), "name", cr.Name, "namespace", cr.Namespace) // Look up the user who made this request req, err := admission.RequestFromContext(ctx) @@ -101,7 +101,7 @@ func (r *CryostatValidator) validate(ctx context.Context, obj runtime.Object, op }, } - err := r.Client.Create(ctx, sar) + err := r.client.Create(ctx, sar) if err != nil { return nil, fmt.Errorf("failed to check permissions: %w", err) } diff --git a/internal/webhooks/validator_test.go b/internal/webhooks/validator_test.go index c370c56d..4d6bcada 100644 --- a/internal/webhooks/validator_test.go +++ b/internal/webhooks/validator_test.go @@ -34,9 +34,8 @@ import ( ) type validatorTestInput struct { - client ctrlclient.Client - validator *webhooks.CryostatValidator - objs []ctrlclient.Object + client ctrlclient.Client + objs []ctrlclient.Object *webhooktests.WebhookTestResources } @@ -46,7 +45,7 @@ var _ = Describe("CryostatValidator", func() { count := 0 namespaceWithSuffix := func(name string) string { - return name + "-" + strconv.Itoa(count) + return name + "-validator-" + strconv.Itoa(count) } BeforeEach(func() { @@ -73,11 +72,6 @@ var _ = Describe("CryostatValidator", func() { logf.SetLogger(logger) t.client = k8sClient - t.validator = &webhooks.CryostatValidator{ - Client: k8sClient, - Log: &logger, - } - for _, obj := range t.objs { err := t.client.Create(ctx, obj) Expect(err).ToNot(HaveOccurred()) From d058b54494c33108d15d44b616a723c067a6d0d9 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Thu, 19 Dec 2024 19:05:02 -0500 Subject: [PATCH 5/6] test(sampleapp): configure Cryostat Agent sample app to use k8s serviceaccount token auth (#983) * test(sampleapp): configure Cryostat Agent sample app to use k8s serviceaccount token auth * allow default 'auto' authorization type * use unique name so deployment can be scaled to multiple replicas --- Makefile | 1 - config/samples/sample-app-agent.yaml | 38 +++++++++++++++++++++++++--- 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index be6b9278..5d9d34fc 100644 --- a/Makefile +++ b/Makefile @@ -415,7 +415,6 @@ sample_app_agent: undeploy_sample_app_agent ## Deploy sample app with Cryostat A fi; \ fi; \ $(CLUSTER_CLIENT) apply $(SAMPLE_APP_FLAGS) -f config/samples/sample-app-agent.yaml; \ - $(CLUSTER_CLIENT) set env $(SAMPLE_APP_FLAGS) deployment/quarkus-cryostat-agent CRYOSTAT_AGENT_AUTHORIZATION="Bearer $(AUTH_TOKEN)" .PHONY: undeploy_sample_app_agent_proxy undeploy_sample_app_agent_proxy: ## Undeploy sample app with Cryostat Agent configured for TLS client auth on nginx proxy. diff --git a/config/samples/sample-app-agent.yaml b/config/samples/sample-app-agent.yaml index ece50f2e..5e6d0efd 100644 --- a/config/samples/sample-app-agent.yaml +++ b/config/samples/sample-app-agent.yaml @@ -15,10 +15,14 @@ spec: labels: app: quarkus-cryostat-agent spec: + serviceAccountName: quarkus-cryostat-agent-serviceaccount containers: - env: - name: CRYOSTAT_AGENT_APP_NAME - value: agent-test + valueFrom: + fieldRef: + apiVersion: v1 + fieldPath: metadata.name - name: NAMESPACE valueFrom: fieldRef: @@ -35,12 +39,11 @@ spec: fieldPath: status.podIP - name: CRYOSTAT_AGENT_CALLBACK value: http://$(POD_IP):9977 - - name: CRYOSTAT_AGENT_AUTHORIZATION - value: Bearer abcd1234 - name: JAVA_OPTS_APPEND value: |- -Dquarkus.http.host=0.0.0.0 -Djava.util.logging.manager=org.jboss.logmanager.LogManager + -Dio.cryostat.agent.shaded.org.slf4j.simpleLogger.defaultLogLevel=info -Dcom.sun.management.jmxremote.port=9097 -Dcom.sun.management.jmxremote.ssl=false -Dcom.sun.management.jmxremote.authenticate=false @@ -102,3 +105,32 @@ spec: port: 10010 protocol: TCP targetPort: 10010 +--- +kind: ServiceAccount +apiVersion: v1 +metadata: + name: quarkus-cryostat-agent-serviceaccount +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: Role +metadata: + name: quarkus-cryostat-agent-role +rules: +- apiGroups: + - "" + verbs: + - create + resources: + - pods/exec +--- +kind: RoleBinding +apiVersion: rbac.authorization.k8s.io/v1 +metadata: + name: quarkus-cryostat-agent-role-binding +subjects: +- kind: ServiceAccount + name: quarkus-cryostat-agent-serviceaccount +roleRef: + kind: Role + name: quarkus-cryostat-agent-role + apiGroup: rbac.authorization.k8s.io From 4ce5ccaada6b51fb8418fa26a35294c4eb9eac74 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Fri, 20 Dec 2024 11:10:09 -0500 Subject: [PATCH 6/6] fix(tls): use fixed-length cert CommonNames (#968) * fix(tls): use fixed-length cert CommonNames * certificate change recreation * delete cert secret so they are also recreated --- ...yostat-operator.clusterserviceversion.yaml | 2 +- internal/controllers/certmanager.go | 25 +++++++- .../resource_definitions/certificates.go | 11 ++-- internal/controllers/constants/constants.go | 6 ++ internal/controllers/reconciler_test.go | 60 +++++++++++++++++++ internal/test/resources.go | 35 +++++++++-- 6 files changed, 125 insertions(+), 14 deletions(-) diff --git a/bundle/manifests/cryostat-operator.clusterserviceversion.yaml b/bundle/manifests/cryostat-operator.clusterserviceversion.yaml index 9aa26834..297b8298 100644 --- a/bundle/manifests/cryostat-operator.clusterserviceversion.yaml +++ b/bundle/manifests/cryostat-operator.clusterserviceversion.yaml @@ -30,7 +30,7 @@ metadata: capabilities: Seamless Upgrades categories: Monitoring, Developer Tools containerImage: quay.io/cryostat/cryostat-operator:4.0.0-dev - createdAt: "2024-10-10T18:16:26Z" + createdAt: "2024-11-05T19:02:47Z" description: JVM monitoring and profiling tool operatorframework.io/initialization-resource: |- { diff --git a/internal/controllers/certmanager.go b/internal/controllers/certmanager.go index 35df44c1..9305c77e 100644 --- a/internal/controllers/certmanager.go +++ b/internal/controllers/certmanager.go @@ -24,6 +24,7 @@ import ( "github.com/cryostatio/cryostat-operator/internal/controllers/common" resources "github.com/cryostatio/cryostat-operator/internal/controllers/common/resource_definitions" "github.com/cryostatio/cryostat-operator/internal/controllers/model" + "github.com/google/go-cmp/cmp" corev1 "k8s.io/api/core/v1" kerrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" @@ -398,25 +399,43 @@ func (r *Reconciler) reconcileAgentCertificate(ctx context.Context, cert *certv1 return nil } +var errCertificateModified error = errors.New("certificate has been modified") + func (r *Reconciler) createOrUpdateCertificate(ctx context.Context, cert *certv1.Certificate, owner metav1.Object) error { - certSpec := cert.Spec.DeepCopy() + certCopy := cert.DeepCopy() op, err := controllerutil.CreateOrUpdate(ctx, r.Client, cert, func() error { if owner != nil { if err := controllerutil.SetControllerReference(owner, cert, r.Scheme); err != nil { return err } } - // Update Certificate spec - cert.Spec = *certSpec + + if cert.CreationTimestamp.IsZero() { + cert.Spec = certCopy.Spec + } else if !cmp.Equal(cert.Spec, certCopy.Spec) { + return errCertificateModified + } + return nil }) if err != nil { + if err == errCertificateModified { + return r.recreateCertificate(ctx, certCopy, owner) + } return err } r.Log.Info(fmt.Sprintf("Certificate %s", op), "name", cert.Name, "namespace", cert.Namespace) return nil } +func (r *Reconciler) recreateCertificate(ctx context.Context, cert *certv1.Certificate, owner metav1.Object) error { + err := r.deleteCertWithSecret(ctx, cert) + if err != nil { + return err + } + return r.createOrUpdateCertificate(ctx, cert, owner) +} + func newKeystoreSecret(cr *model.CryostatInstance) *corev1.Secret { return &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ diff --git a/internal/controllers/common/resource_definitions/certificates.go b/internal/controllers/common/resource_definitions/certificates.go index be757c74..55bf7645 100644 --- a/internal/controllers/common/resource_definitions/certificates.go +++ b/internal/controllers/common/resource_definitions/certificates.go @@ -20,6 +20,7 @@ import ( certv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" certMeta "github.com/cert-manager/cert-manager/pkg/apis/meta/v1" "github.com/cryostatio/cryostat-operator/internal/controllers/common" + "github.com/cryostatio/cryostat-operator/internal/controllers/constants" "github.com/cryostatio/cryostat-operator/internal/controllers/model" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" @@ -62,7 +63,7 @@ func NewCryostatCACert(gvk *schema.GroupVersionKind, cr *model.CryostatInstance) Namespace: cr.InstallNamespace, }, Spec: certv1.CertificateSpec{ - CommonName: fmt.Sprintf("ca.%s.cert-manager", cr.Name), + CommonName: constants.CryostatCATLSCommonName, SecretName: common.ClusterUniqueNameWithPrefix(gvk, "ca", cr.Name, cr.InstallNamespace), IssuerRef: certMeta.ObjectReference{ Name: cr.Name + "-self-signed", @@ -79,7 +80,7 @@ func NewCryostatCert(cr *model.CryostatInstance, keystoreSecretName string) *cer Namespace: cr.InstallNamespace, }, Spec: certv1.CertificateSpec{ - CommonName: fmt.Sprintf("%s.%s.svc", cr.Name, cr.InstallNamespace), + CommonName: constants.CryostatTLSCommonName, DNSNames: []string{ cr.Name, fmt.Sprintf("%s.%s.svc", cr.Name, cr.InstallNamespace), @@ -115,7 +116,7 @@ func NewReportsCert(cr *model.CryostatInstance) *certv1.Certificate { Namespace: cr.InstallNamespace, }, Spec: certv1.CertificateSpec{ - CommonName: fmt.Sprintf("%s-reports.%s.svc", cr.Name, cr.InstallNamespace), + CommonName: constants.ReportsTLSCommonName, DNSNames: []string{ cr.Name + "-reports", fmt.Sprintf("%s-reports.%s.svc", cr.Name, cr.InstallNamespace), @@ -140,7 +141,7 @@ func NewAgentCert(cr *model.CryostatInstance, namespace string, gvk *schema.Grou Namespace: cr.InstallNamespace, }, Spec: certv1.CertificateSpec{ - CommonName: fmt.Sprintf("*.%s.pod", namespace), + CommonName: constants.AgentsTLSCommonName, DNSNames: []string{ fmt.Sprintf("*.%s.pod", namespace), }, @@ -163,7 +164,7 @@ func NewAgentProxyCert(cr *model.CryostatInstance) *certv1.Certificate { Namespace: cr.InstallNamespace, }, Spec: certv1.CertificateSpec{ - CommonName: fmt.Sprintf("%s-agent.%s.svc", cr.Name, cr.InstallNamespace), + CommonName: constants.AgentAuthProxyTLSCommonName, DNSNames: []string{ cr.Name + "-agent", fmt.Sprintf("%s-agent.%s.svc", cr.Name, cr.InstallNamespace), diff --git a/internal/controllers/constants/constants.go b/internal/controllers/constants/constants.go index 59b7bd9b..5b3b26ec 100644 --- a/internal/controllers/constants/constants.go +++ b/internal/controllers/constants/constants.go @@ -50,4 +50,10 @@ const ( targetNamespaceCRLabelPrefix = "operator.cryostat.io/" TargetNamespaceCRNameLabel = targetNamespaceCRLabelPrefix + "name" TargetNamespaceCRNamespaceLabel = targetNamespaceCRLabelPrefix + "namespace" + + CryostatCATLSCommonName = "cryostat-ca-cert-manager" + CryostatTLSCommonName = "cryostat" + ReportsTLSCommonName = "cryostat-reports" + AgentsTLSCommonName = "cryostat-agent" + AgentAuthProxyTLSCommonName = "cryostat-agent-proxy" ) diff --git a/internal/controllers/reconciler_test.go b/internal/controllers/reconciler_test.go index 612c69f4..40a71f33 100644 --- a/internal/controllers/reconciler_test.go +++ b/internal/controllers/reconciler_test.go @@ -1795,6 +1795,66 @@ func (c *controllerTest) commonTests() { t.expectCertificates() }) }) + Context("with modified certificates", func() { + var oldCerts []*certv1.Certificate + BeforeEach(func() { + t.objs = append(t.objs, t.NewCryostat().Object, t.OtherCAIssuer()) + oldCerts = []*certv1.Certificate{ + t.OtherCACert(), + t.OtherAgentProxyCert(), + t.OtherCryostatCert(), + t.OtherReportsCert(), + } + // Add an annotation for each cert, the test will assert that + // the annotation is gone. + for i, cert := range oldCerts { + metav1.SetMetaDataAnnotation(&oldCerts[i].ObjectMeta, "bad", "cert") + t.objs = append(t.objs, cert) + } + }) + JustBeforeEach(func() { + cr := t.getCryostatInstance() + for _, cert := range oldCerts { + // Make the old certs owned by the Cryostat CR + err := controllerutil.SetControllerReference(cr.Object, cert, t.Client.Scheme()) + Expect(err).ToNot(HaveOccurred()) + err = t.Client.Update(context.Background(), cert) + Expect(err).ToNot(HaveOccurred()) + } + t.reconcileCryostatFully() + }) + It("should recreate certificates", func() { + t.expectCertificates() + }) + }) + Context("with a modified certificate TLS CommonName", func() { + var oldCerts []*certv1.Certificate + BeforeEach(func() { + oldCerts = []*certv1.Certificate{ + t.NewCryostatCert(), + t.NewReportsCert(), + t.NewAgentProxyCert(), + } + t.objs = append(t.objs, t.NewCryostat().Object, t.OtherCAIssuer()) + for _, cert := range oldCerts { + t.objs = append(t.objs, cert) + } + }) + JustBeforeEach(func() { + cr := t.getCryostatInstance() + for _, cert := range oldCerts { + // Make the old certs owned by the Cryostat CR + err := controllerutil.SetControllerReference(cr.Object, cert, t.Client.Scheme()) + Expect(err).ToNot(HaveOccurred()) + err = t.Client.Update(context.Background(), cert) + Expect(err).ToNot(HaveOccurred()) + } + t.reconcileCryostatFully() + }) + It("should recreate certificates", func() { + t.expectCertificates() + }) + }) Context("reconciling a multi-namespace request", func() { targetNamespaces := []string{"multi-test-one", "multi-test-two"} diff --git a/internal/test/resources.go b/internal/test/resources.go index bc74bc9f..6c1adf3d 100644 --- a/internal/test/resources.go +++ b/internal/test/resources.go @@ -1053,7 +1053,7 @@ func (r *TestResources) NewCryostatCert() *certv1.Certificate { Namespace: r.Namespace, }, Spec: certv1.CertificateSpec{ - CommonName: fmt.Sprintf(r.Name+".%s.svc", r.Namespace), + CommonName: "cryostat", DNSNames: []string{ r.Name, fmt.Sprintf(r.Name+".%s.svc", r.Namespace), @@ -1084,6 +1084,12 @@ func (r *TestResources) NewCryostatCert() *certv1.Certificate { } } +func (r *TestResources) OtherCryostatCert() *certv1.Certificate { + cert := r.NewCryostatCert() + cert.Spec.CommonName = fmt.Sprintf("%s.%s.svc", r.Name, r.Namespace) + return cert +} + func (r *TestResources) NewReportsCert() *certv1.Certificate { return &certv1.Certificate{ ObjectMeta: metav1.ObjectMeta{ @@ -1091,7 +1097,7 @@ func (r *TestResources) NewReportsCert() *certv1.Certificate { Namespace: r.Namespace, }, Spec: certv1.CertificateSpec{ - CommonName: fmt.Sprintf(r.Name+"-reports.%s.svc", r.Namespace), + CommonName: "cryostat-reports", DNSNames: []string{ r.Name + "-reports", fmt.Sprintf(r.Name+"-reports.%s.svc", r.Namespace), @@ -1110,6 +1116,12 @@ func (r *TestResources) NewReportsCert() *certv1.Certificate { } } +func (r *TestResources) OtherReportsCert() *certv1.Certificate { + cert := r.NewReportsCert() + cert.Spec.CommonName = fmt.Sprintf("%s-reports.%s.svc", r.Name, r.Namespace) + return cert +} + func (r *TestResources) NewAgentProxyCert() *certv1.Certificate { return &certv1.Certificate{ ObjectMeta: metav1.ObjectMeta{ @@ -1117,7 +1129,7 @@ func (r *TestResources) NewAgentProxyCert() *certv1.Certificate { Namespace: r.Namespace, }, Spec: certv1.CertificateSpec{ - CommonName: fmt.Sprintf(r.Name+"-agent.%s.svc", r.Namespace), + CommonName: "cryostat-agent-proxy", DNSNames: []string{ r.Name + "-agent", fmt.Sprintf(r.Name+"-agent.%s.svc", r.Namespace), @@ -1136,6 +1148,12 @@ func (r *TestResources) NewAgentProxyCert() *certv1.Certificate { } } +func (r *TestResources) OtherAgentProxyCert() *certv1.Certificate { + cert := r.NewAgentProxyCert() + cert.Spec.CommonName = fmt.Sprintf("%s-agent.%s.svc", r.Name, r.Namespace) + return cert +} + func (r *TestResources) NewCACert() *certv1.Certificate { return &certv1.Certificate{ ObjectMeta: metav1.ObjectMeta{ @@ -1143,7 +1161,7 @@ func (r *TestResources) NewCACert() *certv1.Certificate { Namespace: r.Namespace, }, Spec: certv1.CertificateSpec{ - CommonName: fmt.Sprintf("ca.%s.cert-manager", r.Name), + CommonName: "cryostat-ca-cert-manager", SecretName: r.getClusterUniqueNameForCA(), IssuerRef: certMeta.ObjectReference{ Name: r.Name + "-self-signed", @@ -1153,6 +1171,13 @@ func (r *TestResources) NewCACert() *certv1.Certificate { } } +func (r *TestResources) OtherCACert() *certv1.Certificate { + cert := r.NewCACert() + cert.Spec.CommonName = fmt.Sprintf("ca.%s.cert-manager", r.Name) + cert.Spec.SecretName = r.Name + "-ca" + return cert +} + func (r *TestResources) NewAgentCert(namespace string) *certv1.Certificate { name := r.getClusterUniqueNameForAgent(namespace) return &certv1.Certificate{ @@ -1161,7 +1186,7 @@ func (r *TestResources) NewAgentCert(namespace string) *certv1.Certificate { Namespace: r.Namespace, }, Spec: certv1.CertificateSpec{ - CommonName: fmt.Sprintf("*.%s.pod", namespace), + CommonName: "cryostat-agent", DNSNames: []string{ fmt.Sprintf("*.%s.pod", namespace), },