Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(api): remove minimal option from CR spec #751

Merged
merged 1 commit into from
Mar 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions api/v1beta1/cryostat_conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ func (src *Cryostat) ConvertTo(dstRaw conversion.Hub) error {
}

func convertSpecTo(src *CryostatSpec, dst *operatorv1beta2.CryostatSpec) {
dst.Minimal = src.Minimal
dst.EnableCertManager = src.EnableCertManager
dst.TrustedCertSecrets = convertCertSecretsTo(src.TrustedCertSecrets)
dst.EventTemplates = convertEventTemplatesTo(src.EventTemplates)
Expand Down Expand Up @@ -326,7 +325,6 @@ func (dst *Cryostat) ConvertFrom(srcRaw conversion.Hub) error {
}

func convertSpecFrom(src *operatorv1beta2.CryostatSpec, dst *CryostatSpec) {
dst.Minimal = src.Minimal
dst.EnableCertManager = src.EnableCertManager
dst.TrustedCertSecrets = convertCertSecretsFrom(src.TrustedCertSecrets)
dst.EventTemplates = convertEventTemplatesFrom(src.EventTemplates)
Expand Down
5 changes: 4 additions & 1 deletion api/v1beta1/cryostat_conversion_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,10 @@ func tableEntriesTo() []TableEntry {
Entry("WS connections", (*test.TestResources).NewCryostatWithWsConnectionsSpecV1Beta1,
(*test.TestResources).NewCryostat),
Entry("command config", (*test.TestResources).NewCryostatWithCommandConfigV1Beta1,
(*test.TestResources).NewCryostatWithIngress))
(*test.TestResources).NewCryostatWithIngress),
Entry("minimal mode", (*test.TestResources).NewCryostatWithMinimalModeV1Beta1,
(*test.TestResources).NewCryostat),
)
}

func tableEntriesFrom() []TableEntry {
Expand Down
3 changes: 0 additions & 3 deletions api/v1beta2/cryostat_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,6 @@ type CryostatSpec struct {
// +optional
// +operator-sdk:csv:customresourcedefinitions:type=spec,order=2
TargetNamespaces []string `json:"targetNamespaces,omitempty"`
// Deploy a pared-down Cryostat instance with no Grafana Dashboard or JFR Data Source.
// +operator-sdk:csv:customresourcedefinitions:type=spec,order=4,displayName="Minimal Deployment",xDescriptors={"urn:alm:descriptor:com.tectonic.ui:booleanSwitch"}
Minimal bool `json:"minimal"`
// List of TLS certificates to trust when connecting to targets.
// +optional
// +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Trusted TLS Certificates"
Expand Down
10 changes: 1 addition & 9 deletions bundle/manifests/cryostat-operator.clusterserviceversion.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ metadata:
"spec": {
"enableCertManager": true,
"eventTemplates": [],
"minimal": false,
"reportOptions": {
"replicas": 0
},
Expand All @@ -54,7 +53,7 @@ metadata:
capabilities: Seamless Upgrades
categories: Monitoring, Developer Tools
containerImage: quay.io/cryostat/cryostat-operator:2.5.0-dev
createdAt: "2024-03-15T21:38:16Z"
createdAt: "2024-03-18T06:34:51Z"
description: JVM monitoring and profiling tool
operatorframework.io/initialization-resource: |-
{
Expand All @@ -65,7 +64,6 @@ metadata:
},
"spec": {
"enableCertManager": true,
"minimal": false,
"reportOptions": {
"replicas": 0
}
Expand Down Expand Up @@ -527,12 +525,6 @@ spec:
path: enableCertManager
x-descriptors:
- urn:alm:descriptor:com.tectonic.ui:booleanSwitch
- description: Deploy a pared-down Cryostat instance with no Grafana Dashboard
or JFR Data Source.
displayName: Minimal Deployment
path: minimal
x-descriptors:
- urn:alm:descriptor:com.tectonic.ui:booleanSwitch
- description: Override default authorization properties for Cryostat on OpenShift.
displayName: Authorization Properties
path: authProperties
Expand Down
6 changes: 0 additions & 6 deletions bundle/manifests/operator.cryostat.io_cryostats.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4844,10 +4844,6 @@ spec:
credentials database.
type: string
type: object
minimal:
description: Deploy a pared-down Cryostat instance with no Grafana
Dashboard or JFR Data Source.
type: boolean
networkOptions:
description: Options to control how the operator exposes the application
outside of the cluster, such as using an Ingress or Route.
Expand Down Expand Up @@ -9056,8 +9052,6 @@ spec:
- secretName
type: object
type: array
required:
- minimal
type: object
status:
description: CryostatStatus defines the observed state of Cryostat.
Expand Down
6 changes: 0 additions & 6 deletions config/crd/bases/operator.cryostat.io_cryostats.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4834,10 +4834,6 @@ spec:
credentials database.
type: string
type: object
minimal:
description: Deploy a pared-down Cryostat instance with no Grafana
Dashboard or JFR Data Source.
type: boolean
networkOptions:
description: Options to control how the operator exposes the application
outside of the cluster, such as using an Ingress or Route.
Expand Down Expand Up @@ -9046,8 +9042,6 @@ spec:
- secretName
type: object
type: array
required:
- minimal
type: object
status:
description: CryostatStatus defines the observed state of Cryostat.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ metadata:
},
"spec": {
"enableCertManager": true,
"minimal": false,
"reportOptions": {
"replicas": 0
}
Expand Down Expand Up @@ -77,12 +76,6 @@ spec:
path: enableCertManager
x-descriptors:
- urn:alm:descriptor:com.tectonic.ui:booleanSwitch
- description: Deploy a pared-down Cryostat instance with no Grafana Dashboard
or JFR Data Source.
displayName: Minimal Deployment
path: minimal
x-descriptors:
- urn:alm:descriptor:com.tectonic.ui:booleanSwitch
- description: Override default authorization properties for Cryostat on OpenShift.
displayName: Authorization Properties
path: authProperties
Expand Down
1 change: 0 additions & 1 deletion config/samples/operator_v1beta2_cryostat.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ kind: Cryostat
metadata:
name: cryostat-sample
spec:
minimal: false
enableCertManager: true
trustedCertSecrets: []
eventTemplates: []
Expand Down
11 changes: 0 additions & 11 deletions docs/config.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,6 @@ When installed in a multi-namespace manner, all users with access to a Cryostat

For now, all authorization checks are done against the namespace where Cryostat is installed. For a user to use Cryostat with workloads in a target namespace, that user must have the necessary Kubernetes permissions in the namespace where Cryostat is installed.

### Minimal Deployment
The `spec.minimal` property determines what is deployed alongside Cryostat. This value is set to `false` by default, which tells the operator to deploy Cryostat, with a [customized Grafana](https://github.com/cryostatio/cryostat-grafana-dashboard) and a [Grafana Data Source for JFR files](https://github.com/cryostatio/jfr-datasource) as 3 containers within a Pod. When `minimal` is set to `true`, the Deployment consists of only the Cryostat container.
```yaml
apiVersion: operator.cryostat.io/v1beta1
kind: Cryostat
metadata:
name: cryostat-sample
spec:
minimal: true
```

### Disabling cert-manager Integration
By default, the operator expects [cert-manager](https://cert-manager.io/) to be available in the cluster. The operator uses cert-manager to generate a self-signed CA to allow traffic between Cryostat components within the cluster to use HTTPS. If cert-manager is not available in the cluster, this integration can be disabled with the `spec.enableCertManager` property.
```yaml
Expand Down
45 changes: 6 additions & 39 deletions internal/controllers/certmanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (
resources "github.com/cryostatio/cryostat-operator/internal/controllers/common/resource_definitions"
"github.com/cryostatio/cryostat-operator/internal/controllers/model"
corev1 "k8s.io/api/core/v1"
kerrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime/schema"
Expand Down Expand Up @@ -95,26 +94,13 @@ func (r *Reconciler) setupTLS(ctx context.Context, cr *model.CryostatInstance) (
}
certificates := []*certv1.Certificate{caCert, cryostatCert, reportsCert}
// Create a certificate for Grafana signed by the Cryostat CA
if !cr.Spec.Minimal {
grafanaCert := resources.NewGrafanaCert(cr)
err = r.createOrUpdateCertificate(ctx, grafanaCert, cr.Object)
if err != nil {
return nil, err
}
certificates = append(certificates, grafanaCert)
tlsConfig.GrafanaSecret = grafanaCert.Spec.SecretName
} else {
grafanaCert := resources.NewGrafanaCert(cr)
secret := secretForCertificate(grafanaCert)
err = r.deleteSecret(ctx, secret)
if err != nil {
return nil, err
}
err = r.deleteCert(ctx, grafanaCert)
if err != nil {
return nil, err
}
grafanaCert := resources.NewGrafanaCert(cr)
err = r.createOrUpdateCertificate(ctx, grafanaCert, cr.Object)
if err != nil {
return nil, err
}
certificates = append(certificates, grafanaCert)
tlsConfig.GrafanaSecret = grafanaCert.Spec.SecretName

// Update owner references of TLS secrets created by cert-manager to ensure proper cleanup
err = r.setCertSecretOwner(ctx, cr.Object, certificates...)
Expand Down Expand Up @@ -211,15 +197,6 @@ func (r *Reconciler) setCertSecretOwner(ctx context.Context, owner metav1.Object
return nil
}

func secretForCertificate(cert *certv1.Certificate) *corev1.Secret {
return &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: cert.Spec.SecretName,
Namespace: cert.Namespace,
},
}
}

func (r *Reconciler) certManagerAvailable() (bool, error) {
// Check if cert-manager API is available. Checking just one should be enough.
_, err := r.RESTMapper.RESTMapping(schema.GroupKind{
Expand Down Expand Up @@ -316,16 +293,6 @@ func (r *Reconciler) createOrUpdateCertSecret(ctx context.Context, secret *corev
return nil
}

func (r *Reconciler) deleteCert(ctx context.Context, cert *certv1.Certificate) error {
err := r.Client.Delete(ctx, cert)
if err != nil && !kerrors.IsNotFound(err) {
r.Log.Error(err, "Could not delete certificate", "name", cert.Name, "namespace", cert.Namespace)
return err
}
r.Log.Info("Cert deleted", "name", cert.Name, "namespace", cert.Namespace)
return nil
}

func (r *Reconciler) getCertficateBytes(ctx context.Context, cert *certv1.Certificate) ([]byte, error) {
secret, err := r.GetCertificateSecret(ctx, cert)
if err != nil {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -237,17 +237,10 @@ func NewDeploymentForReports(cr *model.CryostatInstance, imageTags *ImageTags, t

func NewPodForCR(cr *model.CryostatInstance, specs *ServiceSpecs, imageTags *ImageTags,
tls *TLSConfig, fsGroup int64, openshift bool) *corev1.PodSpec {
var containers []corev1.Container
if cr.Spec.Minimal {
containers = []corev1.Container{
NewCoreContainer(cr, specs, imageTags.CoreImageTag, tls, openshift),
}
} else {
containers = []corev1.Container{
NewCoreContainer(cr, specs, imageTags.CoreImageTag, tls, openshift),
NewGrafanaContainer(cr, imageTags.GrafanaImageTag, tls),
NewJfrDatasourceContainer(cr, imageTags.DatasourceImageTag),
}
containers := []corev1.Container{
NewCoreContainer(cr, specs, imageTags.CoreImageTag, tls, openshift),
NewGrafanaContainer(cr, imageTags.GrafanaImageTag, tls),
NewJfrDatasourceContainer(cr, imageTags.DatasourceImageTag),
}

volumes := newVolumeForCR(cr)
Expand Down Expand Up @@ -296,35 +289,31 @@ func NewPodForCR(cr *model.CryostatInstance, specs *ServiceSpecs, imageTags *Ima
},
})

keyVolume := corev1.Volume{
Name: "keystore",
VolumeSource: corev1.VolumeSource{
Secret: &corev1.SecretVolumeSource{
SecretName: tls.CryostatSecret,
Items: []corev1.KeyToPath{
{
Key: "keystore.p12",
Path: "keystore.p12",
Mode: &readOnlyMode,
volumes = append(volumes,
corev1.Volume{
Name: "keystore",
VolumeSource: corev1.VolumeSource{
Secret: &corev1.SecretVolumeSource{
SecretName: tls.CryostatSecret,
Items: []corev1.KeyToPath{
{
Key: "keystore.p12",
Path: "keystore.p12",
Mode: &readOnlyMode,
},
},
},
},
},
}

volumes = append(volumes, keyVolume)

if !cr.Spec.Minimal {
grafanaSecretVolume := corev1.Volume{
corev1.Volume{
Name: "grafana-tls-secret",
VolumeSource: corev1.VolumeSource{
Secret: &corev1.SecretVolumeSource{
SecretName: tls.GrafanaSecret,
},
},
}
volumes = append(volumes, grafanaSecretVolume)
}
},
)
}

// Project certificate secrets into deployment
Expand Down Expand Up @@ -886,26 +875,24 @@ func NewCoreContainer(cr *model.CryostatInstance, specs *ServiceSpecs, imageTag
},
})

if !cr.Spec.Minimal {
grafanaVars := []corev1.EnvVar{
{
Name: "GRAFANA_DATASOURCE_URL",
Value: datasourceURL,
grafanaVars := []corev1.EnvVar{
{
Name: "GRAFANA_DATASOURCE_URL",
Value: datasourceURL,
},
}
if specs.GrafanaURL != nil {
grafanaVars = append(grafanaVars,
corev1.EnvVar{
Name: "GRAFANA_DASHBOARD_EXT_URL",
Value: specs.GrafanaURL.String(),
},
}
if specs.GrafanaURL != nil {
grafanaVars = append(grafanaVars,
corev1.EnvVar{
Name: "GRAFANA_DASHBOARD_EXT_URL",
Value: specs.GrafanaURL.String(),
},
corev1.EnvVar{
Name: "GRAFANA_DASHBOARD_URL",
Value: getInternalDashboardURL(tls),
})
}
envs = append(envs, grafanaVars...)
corev1.EnvVar{
Name: "GRAFANA_DASHBOARD_URL",
Value: getInternalDashboardURL(tls),
})
}
envs = append(envs, grafanaVars...)

livenessProbeScheme := corev1.URISchemeHTTP
if tls == nil {
Expand Down
5 changes: 2 additions & 3 deletions internal/controllers/ingresses.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,9 @@ func (r *Reconciler) reconcileGrafanaIngress(ctx context.Context, cr *model.Cryo
},
}

if cr.Spec.Minimal || cr.Spec.NetworkOptions == nil || cr.Spec.NetworkOptions.GrafanaConfig == nil ||
if cr.Spec.NetworkOptions == nil || cr.Spec.NetworkOptions.GrafanaConfig == nil ||
cr.Spec.NetworkOptions.GrafanaConfig.IngressSpec == nil {
// User has either chosen a minimal deployment or not requested
// an Ingress, delete if it exists
// User has not requested an Ingress, delete if it exists
return r.deleteIngress(ctx, ingress)
}
grafanaConfig := configureGrafanaIngress(cr)
Expand Down
2 changes: 0 additions & 2 deletions internal/controllers/reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,8 +176,6 @@ func (r *Reconciler) reconcileCryostat(ctx context.Context, cr *model.CryostatIn
}
}

reqLogger.Info("Spec", "Minimal", cr.Spec.Minimal)

// Create lock config map or fail if owned by another CR
err := r.reconcileLockConfigMap(ctx, cr)
if err != nil {
Expand Down
Loading
Loading