Skip to content

Commit

Permalink
Helm Chart Fixes for Env variables and volumes (#35)
Browse files Browse the repository at this point in the history 
* Helm Chart Fixes for Env variables and volumes

The opensearch-dashboards chart failed to render correctly when
utilizing the extraEnvs flag, caused by incorrect indentation.

The opensearch chart failed to render when utlizing the secrets for the
security config, this was due to them being in the env section.

This pull request reqolves both issues, verified via running helm
template with the minumal values files included here:

```yaml
envFrom:
  - secretRef:
      name: kibana-secrets
extraEnvs:
  - name: TENANT_ID
    valueFrom:
      secretKeyRef:
        name: kibana-secrets
        key: tenantID
```

```yaml
securityConfig:
  enabled: true
  configSecret: "security-config"
  internalUsersSecret: "internal-users-config"
  rolesMappingSecret: "roles-mapping-config"
  rolesSecret: "roles-config"
```

Signed-off-by: Harrison Goscenski <[email protected]>

* Updating paths in sts to be dynamic

Updating the paths specified in the sts for opensearch to utilize
.Values.opensearchHome to allow for dynamic paths, with a default of
`/usr/share/opensearch` which should be sufficient for most users.

Signed-off-by: Harrison Goscenski <[email protected]>
  • Loading branch information
hgoscenski-imanage authored Aug 24, 2021
1 parent 4f8fbb3 commit fe831db
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 12 deletions.
2 changes: 1 addition & 1 deletion Helm/opensearch-dashboards/templates/deployment.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ spec:
key: cookie
{{- end }}
{{- if .Values.extraEnvs }}
{{ toYaml .Values.extraEnvs | indent 10 }}
{{ toYaml .Values.extraEnvs | indent 8 }}
{{- end }}
{{- if .Values.envFrom }}
envFrom:
Expand Down
19 changes: 10 additions & 9 deletions Helm/opensearch/templates/statefulset.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ spec:
- name: config
configMap:
name: {{ template "opensearch.uname" . }}-config
{{- end }}
{{- end }}
{{- if and .Values.securityConfig.config.securityConfigSecret .Values.securityConfig.config.data }}
- name: security-config
secret:
Expand Down Expand Up @@ -238,7 +238,7 @@ spec:
echo "$PASSWORD" | opensearch-keystore add -x bootstrap.password
fi
cp -a /usr/share/opensearch/config/opensearch.keystore /tmp/keystore/
cp -a {{ .Values.opensearchHome }}/config/opensearch.keystore /tmp/keystore/
env: {{ toYaml .Values.extraEnvs | nindent 10 }}
envFrom: {{ toYaml .Values.envFrom | nindent 10 }}
resources: {{ toYaml .Values.initResources | nindent 10 }}
Expand Down Expand Up @@ -315,7 +315,12 @@ spec:
volumeMounts:
{{- if .Values.persistence.enabled }}
- name: "{{ template "opensearch.uname" . }}"
mountPath: /usr/share/opensearch/data
mountPath: {{ .Values.opensearchHome }}/data
{{- end }}
{{- if .Values.keystore }}
- name: keystore
mountPath: {{ .Values.opensearchHome }}/config/opensearch.keystore
subPath: opensearch.keystore
{{- end }}
{{- if .Values.securityConfig.enabled }}
{{- if .Values.securityConfig.actionGroupsSecret }}
Expand Down Expand Up @@ -353,11 +358,7 @@ spec:
name: security-config
{{- end }}
{{- end }}
{{ if .Values.keystore }}
- name: keystore
mountPath: /usr/share/opensearch/config/opensearch.keystore
subPath: opensearch.keystore
{{ end }}

{{- range .Values.secretMounts }}
- name: {{ .name }}
mountPath: {{ .path }}
Expand All @@ -367,7 +368,7 @@ spec:
{{- end }}
{{- range $path, $config := .Values.config }}
- name: config
mountPath: /usr/share/opensearch/config/{{ $path }}
mountPath: {{ .Values.opensearchHome }}/config/{{ $path }}
subPath: {{ $path }}
{{- end -}}
{{- if .Values.extraVolumeMounts }}
Expand Down
5 changes: 3 additions & 2 deletions Helm/opensearch/values.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ minimumMasterNodes: 1
majorVersion: ""


# Allows you to add any config files in /usr/share/opensearch/config/
# Allows you to add any config files in {{ .Values.configPath }}/config
opensearchHome: /usr/share/opensearch
# such as opensearch.yml and log4j2.properties
config:
opensearch.yml:
Expand Down Expand Up @@ -65,7 +66,7 @@ config:
enabled: true
indices: [".opendistro-alerting-config", ".opendistro-alerting-alert*", ".opendistro-anomaly-results*", ".opendistro-anomaly-detector*", ".opendistro-anomaly-checkpoints", ".opendistro-anomaly-detection-state", ".opendistro-reports-*", ".opendistro-notifications-*", ".opendistro-notebooks", ".opendistro-asynchronous-search-response*"]
######## End OpenSearch Security Demo Configuration ########
# log4j2.properties:
# log4j2.properties:

# Extra environment variables to append to this nodeGroup
# This will be appended to the current 'env:' key. You can use any of the kubernetes env
Expand Down

7 comments on commit fe831db

@cybernagle
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi , just tested this code with helm , opensearchHome will have issue regarding the range logic.

The failure code is:

          {{- range $path, $config := .Values.config }}
          - name: config
            mountPath: {{ .Values.opensearchHome }}/config/{{ $path }}
            subPath: {{ $path }}
          {{- end -}}

Error Msg:

Error: template: opensearch/templates/statefulset.yaml:371:33: executing "opensearch/templates/statefulset.yaml" at <.Values.opensearchHome>: nil pointer evaluating interface {}.opensearchHome
helm.go:84: [debug] template: opensearch/templates/statefulset.yaml:371:33: executing "opensearch/templates/statefulset.yaml" at <.Values.opensearchHome>: nil pointer evaluating interface {}.opensearchHome

My Helm Version

~/(main*) » helm version
version.BuildInfo{Version:"v3.2.4", GitCommit:"0ad800ef43d3b826f31a5ad8dfbb4fe05d143688", GitTreeState:"clean", GoVersion:"go1.13.12"}

@cybernagle
Copy link
Contributor

Choose a reason for hiding this comment

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

I've try to set a global variable on top of statefulset.yaml
and resolved the issue.

@killerquitsche
Copy link

Choose a reason for hiding this comment

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

Hi @NagleZhang,
Can you give us some more information on how you solved this problem? We have the same error and don't know exactly where to put what to solve this issue. Thanks!

@cybernagle
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @killerquitsche , Just define a variable like below on top of the statefulset.yaml :

{{- $opensearchhome := .Values.opensearchHome | default "/usr/share/opensearch" -}}

Then change

          {{- range $path, $config := .Values.config }}
          - name: config
            mountPath: {{ .Values.opensearchHome }}/config/{{ $path }}
            subPath: {{ $path }}
          {{- end -}}

to

          {{- range $path, $config := .Values.config }}
          - name: config
            mountPath: {{ $opensearchhome  }}/config/{{ $path }}
            subPath: {{ $path }}
          {{- end -}}

@UltimateFighter
Copy link

Choose a reason for hiding this comment

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

To prevent the usage of a global variable you can also write $.Value to access the root scope inside a loop.

{{- range $path, $config := .Values.config }}
- name: config
   mountPath: {{ $.Values.opensearchHome }}/config/{{ $path }}
   subPath: {{ $path }}
{{- end -}}

@cybernagle
Copy link
Contributor

Choose a reason for hiding this comment

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

To prevent the usage of a global variable you can also write $.Value to access the root scope inside a loop.

{{- range $path, $config := .Values.config }}
- name: config
  mountPath: {{ $.Values.opensearchHome }}/config/{{ $path }}
  subPath: {{ $path }}
{{- end -}}

I've verified this code, it works as well , and simpler. thanks a lot.

@killerquitsche
Copy link

Choose a reason for hiding this comment

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

Great! Thank you for your quick help! It works.

Please sign in to comment.