vikunja
/
helm-chart
2
2
Fork 5
Code Issues 6 Pull Requests Packages Projects Releases 1 Wiki Activity

Readd Removed Documentation #19

Merged
konrad merged 4 commits from xeruf/helm-chart:readme-new into main 2024-02-09 18:14:49 +00:00
Conversation 8 Commits 4 Files Changed 2 +58 -38
xeruf commented 2023-12-12 10:44:04 +00:00
Contributor
Copy Link

Readds useful additions from https://kolaente.dev/vikunja/helm-chart/pulls/12 that were somehow discarded with https://kolaente.dev/vikunja/helm-chart/pulls/13/files#diff-8ec9a00bfd09b3190ac6b22251dbb1aa95a0579d

I came back to the chart and had to figure out everything from the beginning once more because the documentation I made last time has been deleted and the configuration vastly changed!

Readds useful additions from https://kolaente.dev/vikunja/helm-chart/pulls/12 that were somehow discarded with https://kolaente.dev/vikunja/helm-chart/pulls/13/files#diff-8ec9a00bfd09b3190ac6b22251dbb1aa95a0579d I came back to the chart and had to figure out everything from the beginning once more because the documentation I made last time has been deleted and the configuration vastly changed!
xeruf added 2 commits 2023-12-12 10:44:04 +00:00
continuous-integration/drone/pr Build is passing Details
207b7493ff Slight replica description adjustment
xeruf added 1 commit 2023-12-12 10:49:11 +00:00
continuous-integration/drone/pr Build is passing Details
450db45e43 Correct api config yaml path
xeruf commented 2023-12-12 10:57:58 +00:00
Author
Contributor
Copy Link

And we should document how to set the domain for ingress, as that is a very common task. I think it should be this?

    frontend:
      image:
        tag: unstable
        pullPolicy: Always
      ingress:
        enabled: true
        annotations:
          kubernetes.io/tls-acme: "true"
        hosts:
          - host: "do-test.${domain}"
            paths:
               - path: /
                 pathType: Prefix
        tls:
          - secretName: vikunja-test-tls
            hosts:
              - "do-test.${domain}"
    api:
      image:
        tag: unstable
        pullPolicy: Always
      ingress:
        enabled: true
        annotations:
          kubernetes.io/tls-acme: "true"
        hosts:
          - host: "do-test.${domain}"
            paths:
               - path: /
                 pathType: Prefix
        tls:
          - secretName: vikunja-test-tls
            hosts:
              - "do-test.${domain}"
And we should document how to set the domain for ingress, as that is a very common task. I think it should be this? ```yaml frontend: image: tag: unstable pullPolicy: Always ingress: enabled: true annotations: kubernetes.io/tls-acme: "true" hosts: - host: "do-test.${domain}" paths: - path: / pathType: Prefix tls: - secretName: vikunja-test-tls hosts: - "do-test.${domain}" api: image: tag: unstable pullPolicy: Always ingress: enabled: true annotations: kubernetes.io/tls-acme: "true" hosts: - host: "do-test.${domain}" paths: - path: / pathType: Prefix tls: - secretName: vikunja-test-tls hosts: - "do-test.${domain}" ```
xeruf commented 2023-12-12 10:59:43 +00:00
Author
Contributor
Copy Link

proxy-body-size is set to 0 to remove the body limit on file uploads
should we really remove the limit completely? Shouldn't we set something lavish like 50MB?

`proxy-body-size is set to 0 to remove the body limit on file uploads` should we really remove the limit completely? Shouldn't we set something lavish like 50MB?
konrad commented 2023-12-12 18:11:29 +00:00
Owner
Copy Link

And we should document how to set the domain for ingress, as that is a very common task. I think it should be this?

Did you test it?

should we really remove the limit completely? Shouldn't we set something lavish like 50MB?

Vikunja itself has a limit set. Removing it in nginx allows us to only set it in Vikunja and not in two places.

> And we should document how to set the domain for ingress, as that is a very common task. I think it should be this? Did you test it? > should we really remove the limit completely? Shouldn't we set something lavish like 50MB? Vikunja itself has a limit set. Removing it in nginx allows us to only set it in Vikunja and not in two places.
konrad reviewed 2023-12-12 18:12:15 +00:00
README.md
@ -14,2 +14,3 @@
You can set all Vikunja API options as yaml under `api.configMaps.config.data.config.yml`: https://vikunja.io/docs/config-options
That should be it!
For example, you can disable registration (if you do not with to allow others to register on your Vikunja), by providing the following values in your `values.yaml`:
konrad commented 2023-12-12 18:12:15 +00:00
Owner
Copy Link

Didn't we already had this and removed it?

Didn't we already had this and removed it?
konrad commented 2023-12-12 18:12:45 +00:00
Owner
Copy Link

I think we should keep both changes

I think we should keep both changes
xeruf commented 2023-12-12 18:32:10 +00:00
Author
Contributor
Copy Link

I just moved it up because specifying API config options is one of the first things you usually want to do, and made the text more understandable and compact.

I just moved it up because specifying API config options is one of the first things you usually want to do, and made the text more understandable and compact.
👍 1
konrad commented 2023-12-12 21:09:57 +00:00
Owner
Copy Link

Makes sense.

Makes sense.
konrad marked this conversation as resolved
konrad reviewed 2023-12-12 21:09:41 +00:00
README.md
@ -16,0 +41,4 @@
by setting `api.configMaps.config.data.config.yml.keyvalue.type` to `redis`,
configuring the redis subchart (see [values.yaml](./values.yaml#L119))
and the connection to Vikunja:
https://vikunja.io/docs/config-options/#redis
konrad commented 2023-12-12 21:09:06 +00:00
Owner
Copy Link

Please do something like

and the connection [in Vikunja](https://vikunja.io/docs/config-options/#redis)
``
Please do something like ``` and the connection [in Vikunja](https://vikunja.io/docs/config-options/#redis) ``
xeruf marked this conversation as resolved
xeruf commented 2023-12-12 21:23:19 +00:00
Author
Contributor
Copy Link

failed to create resource: admission webhook "validate.nginx.ingress.kubernetes.io" denied the request: host "do-test.ftt.gmbh" and path "/" is already defined in ingress stackspout/vikunja-test-api

this is what I get when I test the ingress config above, please provide a working example @perfectra1n

> failed to create resource: admission webhook "validate.nginx.ingress.kubernetes.io" denied the request: host "do-test.ftt.gmbh" and path "/" is already defined in ingress stackspout/vikunja-test-api this is what I get when I test the ingress config above, please provide a working example @perfectra1n
xeruf commented 2023-12-12 21:27:55 +00:00
Author
Contributor
Copy Link

it would be elegant to be able to just set the domain once without all the ingress baggage as discussed in #10

it would be elegant to be able to just set the domain once without all the ingress baggage as discussed in https://kolaente.dev/vikunja/helm-chart/issues/10
konrad commented 2023-12-12 21:29:05 +00:00
Owner
Copy Link

it would be elegant to be able to just set the domain once without all the ingress baggage as discussed in #10

Well I'd merge a PR with that :)

> it would be elegant to be able to just set the domain once without all the ingress baggage as discussed in #10 Well I'd merge a PR with that :)
perfectra1n commented 2023-12-12 21:47:24 +00:00
Contributor
Copy Link

Hey there @xeruf! I'll certainly add this to the docs, but since the common library is being used, any values that can be found within applications at kubesearch.dev could be very similarly used as values for this Helm chart.

I'm not sure if having +1 additional ingress can be considered "baggage", but there are ways you could achieve that goal.

Using two ingresses (allow each release to manage their own Ingress object, this method is also simpler as the chart automagically associates the created Service objects with the created Ingress):

frontend:
  image:
    tag: unstable
    pullPolicy: Always
  ingress:
    main:
      enabled: true
      annotations:
        kubernetes.io/tls-acme: 'true'
      hosts:
        - host: 'do-test.${domain}'
          paths:
          - path: /
            pathType: Prefix
              
      tls:
        - secretName: vikunja-test-tls
          hosts:
            - 'do-test.${domain}'
api:
  image:
    tag: unstable
    pullPolicy: Always
  ingress:
    main:
      enabled: true
      annotations:
        kubernetes.io/tls-acme: 'true'
      hosts:
        - host: 'do-test.${domain}'
          paths:
          # Since this is the API container, we only want to forward `/api` calls to it
            - path: /api/v1
              pathType: Prefix

      tls:
        - secretName: vikunja-test-tls
          hosts:
            - 'do-test.${domain}'

Then only having a single ingress (you just have to specify the Service name that you wish to forward the Ingress traffic to):

frontend:
  image:
    tag: unstable
    pullPolicy: Always
  ingress:
    main:
      enabled: true
      annotations:
        kubernetes.io/tls-acme: 'true'
      hosts:
        - host: 'do-test.${domain}'
          paths:
            - path: /
              pathType: Prefix
              service:
                name: "vikunja-test-frontend"
                port: 80
          # Point the ingress path to the service that will be created
            - path: /api/v1
              pathType: Prefix
              service:
                name: "vikunja-test-api"
                port: 3456
      tls:
        - secretName: vikunja-test-tls
          hosts:
          - 'do-test.${domain}'
api:
  image:
    tag: unstable
    pullPolicy: Always
  ingress:
    main:
      enabled: false

I'll add these examples to the documentation too, I can make a PR once this one is closed?

Hey there @xeruf! I'll certainly add this to the docs, but since the `common` library is being used, any values that can be found within applications at [kubesearch.dev](https://kubesearch.dev/#bjw-s) could be very similarly used as values for this Helm chart. I'm not sure if having +1 additional ingress can be considered "baggage", but there are ways you could achieve that goal. Using two ingresses (allow each release to manage their own `Ingress` object, this method is also simpler as the chart automagically associates the created `Service` objects with the created `Ingress`): ```yaml frontend: image: tag: unstable pullPolicy: Always ingress: main: enabled: true annotations: kubernetes.io/tls-acme: 'true' hosts: - host: 'do-test.${domain}' paths: - path: / pathType: Prefix tls: - secretName: vikunja-test-tls hosts: - 'do-test.${domain}' api: image: tag: unstable pullPolicy: Always ingress: main: enabled: true annotations: kubernetes.io/tls-acme: 'true' hosts: - host: 'do-test.${domain}' paths: # Since this is the API container, we only want to forward `/api` calls to it - path: /api/v1 pathType: Prefix tls: - secretName: vikunja-test-tls hosts: - 'do-test.${domain}' ``` Then only having a single ingress (you just have to specify the `Service` name that you wish to forward the `Ingress` traffic to): ```yaml frontend: image: tag: unstable pullPolicy: Always ingress: main: enabled: true annotations: kubernetes.io/tls-acme: 'true' hosts: - host: 'do-test.${domain}' paths: - path: / pathType: Prefix service: name: "vikunja-test-frontend" port: 80 # Point the ingress path to the service that will be created - path: /api/v1 pathType: Prefix service: name: "vikunja-test-api" port: 3456 tls: - secretName: vikunja-test-tls hosts: - 'do-test.${domain}' api: image: tag: unstable pullPolicy: Always ingress: main: enabled: false ``` I'll add these examples to the documentation too, I can make a PR once this one is closed?
xeruf added 1 commit 2024-02-08 15:56:15 +00:00
continuous-integration/drone/pr Build is passing Details
5762408832 Link artifacthub in readme
xeruf commented 2024-02-08 15:56:34 +00:00
Author
Contributor
Copy Link

time to merge this, and let's continue the ingress discussion in the other PR :)

time to merge this, and let's continue the ingress discussion in the other PR :)
konrad merged commit 8300c7599f into main 2024-02-09 18:14:49 +00:00
konrad deleted branch readme-new 2024-02-09 18:14:50 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
bug

Something is not working

  
duplicate

This issue or pull request already exists

  
enhancement

New feature

  
help wanted

Need some help

  
invalid

Something is wrong

  
question

More information is needed

  
wontfix

This won't be fixed

No Label
bug
duplicate
enhancement
help wanted
invalid
question
wontfix
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: vikunja/helm-chart#19
No description provided.
Delete Branch "xeruf/helm-chart:readme-new"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?