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

fix(nextcloud): truly not require SSE-C #643

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

jstewart612
Copy link

@jstewart612 jstewart612 commented Oct 6, 2024

Description of the change

Make SSE-C truly not be required just as the upstream project does not require SSE-C.

Benefits

Make SSE-C truly not be required just as the upstream project does not require SSE-C.

Possible drawbacks

None: only makes Helm chart more compliant with available upstream options.

Applicable issues

Additional information

Checklist

jstewart612 and others added 2 commits October 6, 2024 19:29
@jessebot jessebot self-requested a review October 20, 2024 10:49
Copy link
Collaborator

@jessebot jessebot left a comment

Choose a reason for hiding this comment

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

Looks good, can be uncomplicated just a bit though :)

@@ -307,7 +307,8 @@ S3 as primary object store env vars
secretKeyRef:
name: {{ .Values.nextcloud.objectStore.s3.existingSecret }}
key: {{ .Values.nextcloud.objectStore.s3.secretKeys.sse_c_key }}
{{- else }}
{{- end }}
{{- if and (gt (len .Values.nextcloud.objectStore.s3.sse_c_key) 0) (lt (len .Values.nextcloud.objectStore.s3.secretKeys.sse_c_key) 1) }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
{{- if and (gt (len .Values.nextcloud.objectStore.s3.sse_c_key) 0) (lt (len .Values.nextcloud.objectStore.s3.secretKeys.sse_c_key) 1) }}
{{- if .Values.nextcloud.objectStore.s3.sse_c_key }}

We don't actually need to calculate the length, as if this is not an empty string, it will trigger. My above suggestion should work fine.

@jessebot jessebot added 2. developing Work in progress S3 Anything to do with S3 object storage labels Oct 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2. developing Work in progress S3 Anything to do with S3 object storage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants