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

Implement spec.uid for GrafanaFolder #1686

Merged
merged 2 commits into from
Oct 16, 2024
Merged

Conversation

Baarsgaard
Copy link
Contributor

@Baarsgaard Baarsgaard commented Sep 22, 2024

Disclaimer: I'm not a Go developer in any capacity, feel free to address any issues as I will gladly modify the PR to suit the project.

I took a stab at #1636 and limited it to the GrafanaFolder resource for now, I'm planning a separate PR for both GrafanaNotificationPolicy and GrafanaContactPoint with almost identical changes once the design in this PR has been approved.

The Makefile changes were mainly due to me trying to figure out how to work with the Operator locally and I found a lot of discrepancies with the make help target.

I struggled a fair bit with testing and validating as I'm unfamiliar with the project, hence I spent a lot of time in the Makefile.
For now, I have yet to find an existing way to easily deploy the most recently built ko-image into my local kind cluster, specifically due to the default imagePullPolicy: Always that is default in the config/manager/manager.yaml

Local procedure for testing
make start-kind
make install
make ko-build-kind
IMG=ko.local/grafana/grafana-operator make deploy
# Change imagePullPolicy
kubectl patch deploy -n grafana-operator-system grafana-operator-controller-manager-v5  --type='json' -p='[{"op": "replace", "path": "/spec/template/spec/containers/0/imagePullPolicy", "value":"IfNotPresent"}]'

# Test resources
kubectl apply -f https://raw.githubusercontent.com/grafana-operator/grafana-operator/master/examples/basic/resources.yaml
kubectl apply -f ../test-folders.yaml

../test-folders.yaml

apiVersion: grafana.integreatly.org/v1beta1
kind: GrafanaFolder
metadata:
  name: test-folder
spec:
  instanceSelector:
    matchLabels:
      dashboards: grafana
  title: Folder
---
apiVersion: grafana.integreatly.org/v1beta1
kind: GrafanaFolder
metadata:
  name: test-folder2
spec:
  instanceSelector:
    matchLabels:
      dashboards: grafana
  # uid: custom-uid # I repeatedly changed this to ensure immutability rules worked as expected
  title: Folder2
  • Changing the uid: The GrafanaFolder "test-folder2" is invalid: spec.uid: Invalid value: "string": Value is immutable
  • Adding/Removing uid: The GrafanaFolder "test-folder2" is invalid: spec: Invalid value: "object": uid is immutable
Old vs new make help
Usage:
  make <target>

General
  help             Display this help.
<  yq               Download yq locally if necessary.

Development
  manifests        Generate WebhookConfiguration, ClusterRole and CustomResourceDefinition objects.
  generate         Generate code containing DeepCopy, DeepCopyInto, and DeepCopyObject method implementations.
  vet              Run go vet against code.
  test             Run tests.

Build
  build            Build manager binary.
  run              Run a controller from your host.

Deployment
  install          Install CRDs into the K8s cluster specified in ~/.kube/config.
  uninstall        Uninstall CRDs from the K8s cluster specified in ~/.kube/config. Call with ignore-not-found=true to ignore resource not found errors during deletion.
  deploy           Deploy controller to the K8s cluster specified in ~/.kube/config.
  deploy-chainsaw  Deploy controller to the K8s cluster specified in ~/.kube/config.
  undeploy         Undeploy controller from the K8s cluster specified in ~/.kube/config. Call with ignore-not-found=true to ignore resource not found errors during deletion.
>  start-kind       Start kind cluster locally

Build Dependencies
  kustomize        Download kustomize locally if necessary.
  controller-gen   Download controller-gen locally if necessary.
  envtest          Download envtest-setup locally if necessary.
  operator-sdk     Download operator-sdk locally if necessary.
>  yq               Download yq locally if necessary.
>  kind             Download kind locally if necessary.
  bundle           Generate bundle manifests and metadata, then validate generated files.
  e2e              Run e2e tests using chainsaw.
>  ko-build-local   Build Docker image with KO
>  ko-build-kind    Build and Load Docker image into kind cluster
  bundle-build     Build the bundle image.
  bundle-push      Push the bundle image.
  opm              Download opm locally if necessary.
  catalog-build    Build a catalog image.
  catalog-push     Push a catalog image.

@Baarsgaard Baarsgaard marked this pull request as ready for review September 22, 2024 17:44
@aboulay-numspot
Copy link
Contributor

Hello @Baarsgaard,

I have played the following scenarios on my local cluster. I work pretty well!

Here is the list of played scenarios:
✅ create folder without uid
✅ create folder with uid
✅ create folder in a parent folder with uid
✅ update the parent folder of the folder with uid
✅ update the uid of the resource with already an uid (should fail)
✅ remove the uid of the resource (should fail)
✅ add an uid to an existing folder without uid (should fail)

I have try to work myself on it and I was blocking into the two last point. Well done!

api/v1beta1/grafanafolder_types.go Outdated Show resolved Hide resolved
@theSuess theSuess enabled auto-merge October 16, 2024 07:22
@theSuess theSuess added this pull request to the merge queue Oct 16, 2024
Merged via the queue into grafana:master with commit 0100aa2 Oct 16, 2024
14 checks passed
@Rohlik
Copy link

Rohlik commented Nov 8, 2024

Getting an error while I am trying to use spec.uid within GrafanaFolder:

GrafanaFolder test-folder is invalid: problem validating schema. Check JSON formatting: jsonschema: '/spec' does not validate with https://raw.githubusercontent.com/datreeio/CRDs-catalog/main/grafana.integreatly.org/grafanafolder_v1beta1.json#/properties/spec/additionalProperties: additionalProperties 'uid' not allowed

@theSuess
Copy link
Member

theSuess commented Nov 8, 2024

We do not control the CRDs-catalog. You'll have to create an issue or PR there to update the CRDs based on our latest github release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants