Warning when setting a namespaces location in keto

rubenv commented 1 year ago

Preflight checklist

[X] I could not find a solution in the existing issues, docs, nor discussions.
[X] I agree to follow this project's Code of Conduct.
[X] I have read and am following this repository's Contribution Guidelines.
[ ] This issue affects my Ory Network project.
[ ] I have joined the Ory Community Slack.
[ ] I am signed up to the Ory Security Patch Newsletter.

Describe the bug

When setting the location of an OPL file through values, I get:

coalesce.go:223: warning: destination for keto.keto.config.namespaces is a table. Ignoring non-table value  ([map[id:0 name:sample]])

Reproducing the bug

Set the following in values.yaml:

  keto:
    config:
      namespaces:
        location: file:///etc/keto/namespaces.keto.ts

Relevant log output

No response

Relevant configuration

No response

Version

Chart 0.33.4

On which operating system are you observing this issue?

Linux

In which environment are you deploying?

Kubernetes with Helm

Additional Context

No response

Demonsthere commented 1 year ago

Hello there, Namespaces are an array in the format:

namespaces:
  - name: videos
  - name: file:///etc/keto/namespaces.keto.ts

rubenv commented 1 year ago

@Demonsthere That does not work sadly, if I do that, rather than loading the file I end up with a namespace named like that.

With:

 keto:
    config:
      namespaces:
        location: file:///etc/keto/namespaces.keto.ts

I get:

curl localhost:8082/namespaces
{"namespaces":[{"name":"User"},{"name":"Group"}]}

With

 keto:
    config:
      namespaces:
        - name: file:///etc/keto/namespaces.keto.ts

I get:

curl localhost:8082/namespaces
{"namespaces":[{"name":"file:///etc/keto/namespaces.keto.ts"}]}

As you can see here, it can be an array or an object: https://github.com/ory/keto/blob/e7b107cb57ab2f8f3f2800ff06bab198e433c7f4/embedx/config.schema.json#L320-L362

Demonsthere commented 1 year ago

Oh i see, the validation is on the chart level when you are applying overrides. 🤔 The warning is a bit misleading, as the value is passed down to the cm and the application

$ kubectl get cm keto-1688548613-migrate -o yaml                                                                                                                                  
apiVersion: v1
data:
  keto.yaml: |
    namespaces:
      location: file:///etc/keto/namespaces.keto.ts
    serve:
      metrics:
        port: 4468
      read:
        port: 4466
      write:
        port: 4467
kind: ConfigMap
metadata:
  annotations:
    helm.sh/hook: pre-install, pre-upgrade
    helm.sh/hook-delete-policy: before-hook-creation
    helm.sh/hook-weight: "0"
  creationTimestamp: "2023-07-05T09:16:54Z"
  labels:
    app.kubernetes.io/instance: keto-1688548613
    app.kubernetes.io/managed-by: Helm
    app.kubernetes.io/name: keto
    app.kubernetes.io/version: v0.11.0
    helm.sh/chart: keto-0.33.4
    ory.sh/watcher: keto
  name: keto-1688548613-migrate
  namespace: default
  resourceVersion: "1038"
  uid: 9fd173d4-b2ed-4850-839e-fba9b2e1f8f6

But I don't think we can remove it 😞, as helm does not support values of oneOf type. We would need to add type specific fields and functions to parse it and then merge with the rest of the config, which seems a bit of a overkill in order to remove one warning msg

github-actions[bot] commented 4 months ago

Hello contributors!

I am marking this issue as stale as it has not received any engagement from the community or maintainers for a year. That does not imply that the issue has no merit! If you feel strongly about this issue

open a PR referencing and resolving the issue;
leave a comment on it and discuss ideas on how you could contribute towards resolving it;
leave a comment and describe in detail why this issue is critical for your use case;
open a new issue with updated details and a plan for resolving the issue.

Throughout its lifetime, Ory has received over 10.000 issues and PRs. To sustain that growth, we need to prioritize and focus on issues that are important to the community. A good indication of importance, and thus priority, is activity on a topic.

Unfortunately, burnout has become a topic of concern amongst open-source projects.

It can lead to severe personal and health issues as well as opening catastrophic attack vectors.

The motivation for this automation is to help prioritize issues in the backlog and not ignore, reject, or belittle anyone.

If this issue was marked as stale erroneously you can exempt it by adding the backlog label, assigning someone, or setting a milestone for it.

Thank you for your understanding and to anyone who participated in the conversation! And as written above, please do participate in the conversation if this topic is important to you!

Thank you 🙏✌️

Demonsthere commented 3 months ago

closing as won't fix

ory / k8s