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

Add UserContent mechanism #676

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

timmjd
Copy link

@timmjd timmjd commented Aug 3, 2022

What this PR does / why we need it

Implements a way to provide User Content files that will be served by Jenkins. Common use-case is to let Jenkins host a svg or png together with a custom css to customize the UI.

Which issue this PR fixes

Special notes for your reviewer

Checklist

  • DCO signed
  • Chart Version bumped
  • CHANGELOG.md was updated

@timmjd timmjd requested a review from a team as a code owner August 3, 2022 17:55
@timmjd timmjd force-pushed the feature/userContent branch 2 times, most recently from 02a7aed to e360ba2 Compare August 3, 2022 17:57
Copy link
Member

@torstenwalter torstenwalter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if this is the best way to achieve it. Wouldn't it be simpler to mount a config map or any other volume to that location?

I guess that's already possible, but it would not hurt to document in this PR how that works. From my perspective it's not strictly necessary that that volume is managed by this chart.

@timmjd
Copy link
Author

timmjd commented Aug 3, 2022

The use-case I'd like to solve is to let Jenkins host a simple svg or png together with a custom css to adapt the styling of the UI - and that seems to be a quiet common use-case:

To achieve this with the current implementation:

  • Use the persistence.volumes & persistence.mounts option
    • Would require an external ConfigMap / Secret / PVC to be created outside the Helm context
  • Create a custom Docker image where the content gets baked in
    • Would require to build & host the docker image: Additional CI & Hosting infrastructure
  • Download the resources from an external source
    • Requires to host the content via a webserver infrastructure
  • Write a plugin to host the content
    • Total overkill

All of these come with more or less overhead. Therefore, letting Jenkins host the files itself would be the most elegant one. Not sure if there is a more easy way to achieve this - any hint would be great.

@timmjd timmjd force-pushed the feature/userContent branch 2 times, most recently from 60b22b8 to 6088563 Compare August 3, 2022 21:55
Signed-off-by: Timm Drevensek <508684+timmjd@users.noreply.github.com>
@timmjd
Copy link
Author

timmjd commented Aug 4, 2022

Hey @torstenwalter, I refactored to work with ConfigMaps instead of Secrets. Also it's possible to provide UTF-8 or binary encoded content separately. What's your suggestion?

Copy link
Member

@torstenwalter torstenwalter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

content looks ok. Would you mind adding a test for this?

https://github.com/jenkinsci/helm-charts/tree/main/charts/jenkins/unittests has quite some examples

Comment on lines +291 to +304
{{- if or .Values.controller.userContents .Values.controller.binaryUserContents }}
{{- range $content := .Values.controller.userContents }}
- name: jenkins-user-contents
mountPath: {{ $.Values.controller.jenkinsHome }}/userContent/{{ $content.name }}
subPath: {{ $content.name }}
readOnly: true
{{- end }}
{{- range $content := .Values.controller.binaryUserContents }}
- name: jenkins-user-contents
mountPath: {{ $.Values.controller.jenkinsHome }}/userContent/{{ $content.name }}
subPath: {{ $content.name }}
readOnly: true
{{- end }}
{{- end }}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason why you are mounting files individually instead of mounting all at once?

      - name: jenkins-user-contents
        mountPath: {{ $.Values.controller.jenkinsHome }}/userContent/
        readOnly: true

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you mount the Configmap, K8S will create symlinks foobar.txt -> ..data/foobar.txt where ..data -> ..2020_03_03_16_40_36.675612011

Jenkins seems to be not supporting symlinks at this point - neither by displaying nor serving it:
image
Might be related to Jenkins Security Advisory 2021-01-13?

So you can not access userContent/foobar.txt directly but would need to go via something like userContent/..2020_03_03_16_40_36.675612011/foobar.txt - and this is an always changing URL.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thx for the clarification.

@torstenwalter
Copy link
Member

torstenwalter commented Nov 24, 2022

@timmjd Sorry I lost track of this. You can ping me once the merge conflicts are resolved and a test is added otherwise this looks good to me.

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.

2 participants