)]}'
{"cinder/templates/configmap-etc.yaml":[{"author":{"_account_id":24780,"name":"Sangeet Gupta","email":"sg774j@att.com","username":"sgupta"},"change_message_id":"8b0e55117ba9e14ec61c1671ab185becf067a242","unresolved":true,"context_lines":[{"line_number":59,"context_line":"{{- else if eq $dbEngine \"postgresql\" -}}"},{"line_number":60,"context_line":"{{- $_ :\u003d (printf \"%s?sslmode\u003dverify-full\u0026sslrootcert\u003d/etc/mysql/certs/ca.crt\u0026sslkey\u003d/etc/mysql/certs/tls.key\u0026sslcert\u003d/etc/mysql/certs/tls.crt\" $connection ) | set .Values.conf.cinder.database \"connection\" -}}"},{"line_number":61,"context_line":"{{- else -}}"},{"line_number":62,"context_line":"{{- $_ :\u003d set .Values.conf.cinder.database \"connection\" $connection -}}"},{"line_number":63,"context_line":"{{- end -}}"},{"line_number":64,"context_line":"{{- else -}}"},{"line_number":65,"context_line":"{{- $_ :\u003d set .Values.conf.cinder.database \"connection\" $connection -}}"}],"source_content_type":"text/x-yaml","patch_set":12,"id":"51fbff3c_bfedbe25","line":62,"updated":"2021-03-08 14:31:55.000000000","message":"Instead of line 61-66, you can do {{- $_ :\u003d set .Values.conf.cinder.database \"connection\" $connection -}} on line 56 and overwrite connection values in case of 59 or 57.","commit_id":"48583bbaa4132d224fd8e94c612acdca315430f2"}],"cinder/templates/cron-job-cinder-volume-usage-audit.yaml":[{"author":{"_account_id":24780,"name":"Sangeet Gupta","email":"sg774j@att.com","username":"sgupta"},"change_message_id":"8b0e55117ba9e14ec61c1671ab185becf067a242","unresolved":true,"context_lines":[{"line_number":57,"context_line":"            {{ .Values.labels.job.node_selector_key }}: {{ .Values.labels.job.node_selector_value }}"},{"line_number":58,"context_line":"          initContainers:"},{"line_number":59,"context_line":"{{ tuple $envAll \"volume_usage_audit\" $mounts_cinder_volume_usage_audit_init | include \"helm-toolkit.snippets.kubernetes_entrypoint_init_container\" | indent 12 }}"},{"line_number":60,"context_line":"{{- if and $envAll.Values.manifests.certificates $dbTlsSecret }}"},{"line_number":61,"context_line":"{{- $destUid :\u003d default 0 (index (default (dict) (index (dict \"envAll\" $envAll \"application\" \"volume_usage_audit\" | include \"helm-toolkit.snippets.kubernetes_pod_security_context\" | fromYaml) \"securityContext\")) \"runAsUser\") -}}"},{"line_number":62,"context_line":"{{- $destGid :\u003d default 0 (index (default (dict) (index (dict \"envAll\" $envAll \"application\" \"volume_usage_audit\" | include \"helm-toolkit.snippets.kubernetes_pod_security_context\" | fromYaml) \"securityContext\")) \"runAsGroup\") -}}"},{"line_number":63,"context_line":"{{- dict \"uid\" $destUid \"gid\" $destGid \"tlsSecret\" $dbTlsSecret \"targetVolume\" $dbTlsVolume | include \"helm-toolkit.snippets.tls_owner_fix_init_container\" | indent 12 }}"}],"source_content_type":"text/x-yaml","patch_set":12,"id":"3d92689e_6ce34be4","line":60,"range":{"start_line":60,"start_character":4,"end_line":60,"end_character":62},"updated":"2021-03-08 14:31:55.000000000","message":"Why is both certificates and dbTlsSecret checking required everywhere. Can you think of case where certificates is true and there will be no secret for database? If this can happen then we are already in trouble.","commit_id":"48583bbaa4132d224fd8e94c612acdca315430f2"},{"author":{"_account_id":32871,"name":"Ksawery Dziekoński","email":"ksdziekonski@gmail.com","username":"ksdziekonski"},"change_message_id":"adf146df4060b80d594166f55e71405bef731f72","unresolved":true,"context_lines":[{"line_number":57,"context_line":"            {{ .Values.labels.job.node_selector_key }}: {{ .Values.labels.job.node_selector_value }}"},{"line_number":58,"context_line":"          initContainers:"},{"line_number":59,"context_line":"{{ tuple $envAll \"volume_usage_audit\" $mounts_cinder_volume_usage_audit_init | include \"helm-toolkit.snippets.kubernetes_entrypoint_init_container\" | indent 12 }}"},{"line_number":60,"context_line":"{{- if and $envAll.Values.manifests.certificates $dbTlsSecret }}"},{"line_number":61,"context_line":"{{- $destUid :\u003d default 0 (index (default (dict) (index (dict \"envAll\" $envAll \"application\" \"volume_usage_audit\" | include \"helm-toolkit.snippets.kubernetes_pod_security_context\" | fromYaml) \"securityContext\")) \"runAsUser\") -}}"},{"line_number":62,"context_line":"{{- $destGid :\u003d default 0 (index (default (dict) (index (dict \"envAll\" $envAll \"application\" \"volume_usage_audit\" | include \"helm-toolkit.snippets.kubernetes_pod_security_context\" | fromYaml) \"securityContext\")) \"runAsGroup\") -}}"},{"line_number":63,"context_line":"{{- dict \"uid\" $destUid \"gid\" $destGid \"tlsSecret\" $dbTlsSecret \"targetVolume\" $dbTlsVolume | include \"helm-toolkit.snippets.tls_owner_fix_init_container\" | indent 12 }}"}],"source_content_type":"text/x-yaml","patch_set":12,"id":"cd5bb31d_2970e783","line":60,"range":{"start_line":60,"start_character":4,"end_line":60,"end_character":62},"in_reply_to":"3d92689e_6ce34be4","updated":"2021-03-08 15:25:31.000000000","message":"`manifests.certificates` assumes that the database TLS secret is set (let alone set properly), but that secret is created as part of another chart\u0027s scope and, unless value overrides are used consistently, there\u0027s a chance of the secret\u0027s name not being populated.\n\nWhile we can\u0027t easily defend against improperly set values at render time, we can at least do a simple null check for it\u0027s population.","commit_id":"48583bbaa4132d224fd8e94c612acdca315430f2"},{"author":{"_account_id":24780,"name":"Sangeet Gupta","email":"sg774j@att.com","username":"sgupta"},"change_message_id":"edf199ca64709232d03678546235c118b116c4c8","unresolved":true,"context_lines":[{"line_number":57,"context_line":"            {{ .Values.labels.job.node_selector_key }}: {{ .Values.labels.job.node_selector_value }}"},{"line_number":58,"context_line":"          initContainers:"},{"line_number":59,"context_line":"{{ tuple $envAll \"volume_usage_audit\" $mounts_cinder_volume_usage_audit_init | include \"helm-toolkit.snippets.kubernetes_entrypoint_init_container\" | indent 12 }}"},{"line_number":60,"context_line":"{{- if and $envAll.Values.manifests.certificates $dbTlsSecret }}"},{"line_number":61,"context_line":"{{- $destUid :\u003d default 0 (index (default (dict) (index (dict \"envAll\" $envAll \"application\" \"volume_usage_audit\" | include \"helm-toolkit.snippets.kubernetes_pod_security_context\" | fromYaml) \"securityContext\")) \"runAsUser\") -}}"},{"line_number":62,"context_line":"{{- $destGid :\u003d default 0 (index (default (dict) (index (dict \"envAll\" $envAll \"application\" \"volume_usage_audit\" | include \"helm-toolkit.snippets.kubernetes_pod_security_context\" | fromYaml) \"securityContext\")) \"runAsGroup\") -}}"},{"line_number":63,"context_line":"{{- dict \"uid\" $destUid \"gid\" $destGid \"tlsSecret\" $dbTlsSecret \"targetVolume\" $dbTlsVolume | include \"helm-toolkit.snippets.tls_owner_fix_init_container\" | indent 12 }}"}],"source_content_type":"text/x-yaml","patch_set":12,"id":"4e51c3e7_3b760642","line":60,"range":{"start_line":60,"start_character":4,"end_line":60,"end_character":62},"in_reply_to":"cd5bb31d_2970e783","updated":"2021-03-10 14:38:20.000000000","message":"I agree with you but in the case as well if TLS secret is not set we are not throwing error, we are moving along assuming TLS is not enabled. In this case the deployment will still fail because in many places when certificates is true it assumes proper configuration has been provided. As you rightly said \"we can\u0027t easily defend against improperly set values at render time\". Even if we pass here, we will fail somewhere else.","commit_id":"48583bbaa4132d224fd8e94c612acdca315430f2"}],"cinder/templates/job-bootstrap.yaml":[{"author":{"_account_id":24780,"name":"Sangeet Gupta","email":"sg774j@att.com","username":"sgupta"},"change_message_id":"8b0e55117ba9e14ec61c1671ab185becf067a242","unresolved":true,"context_lines":[{"line_number":14,"context_line":""},{"line_number":15,"context_line":"{{- if and .Values.manifests.job_bootstrap .Values.bootstrap.enabled }}"},{"line_number":16,"context_line":"{{- $bootstrapJob :\u003d dict \"envAll\" . \"serviceName\" \"cinder\" \"keystoneUser\" .Values.bootstrap.ks_user \"logConfigFile\" .Values.conf.cinder.DEFAULT.log_config_append -}}"},{"line_number":17,"context_line":"{{- if and .Values.manifests.certificates (index .Values.endpoints.identity.auth .Values.bootstrap.ks_user \"cacert\") -}}"},{"line_number":18,"context_line":"{{- $_ :\u003d set $bootstrapJob \"tlsSecret\" .Values.secrets.tls.volume.api.internal -}}"},{"line_number":19,"context_line":"{{- end -}}"},{"line_number":20,"context_line":"{{ $bootstrapJob | include \"helm-toolkit.manifests.job_bootstrap\" }}"}],"source_content_type":"text/x-yaml","patch_set":12,"id":"3d0a8e63_c0e0e350","line":17,"range":{"start_line":17,"start_character":42,"end_line":17,"end_character":120},"updated":"2021-03-08 14:31:55.000000000","message":"Not sure what value is this adding. If this is not there, our system is already messed up and the deployment needs to be fixed. If we pass here the Kubernetes deployment will fail anyways.\nThis is true for the check that you have added everywhere.","commit_id":"48583bbaa4132d224fd8e94c612acdca315430f2"},{"author":{"_account_id":32871,"name":"Ksawery Dziekoński","email":"ksdziekonski@gmail.com","username":"ksdziekonski"},"change_message_id":"adf146df4060b80d594166f55e71405bef731f72","unresolved":true,"context_lines":[{"line_number":14,"context_line":""},{"line_number":15,"context_line":"{{- if and .Values.manifests.job_bootstrap .Values.bootstrap.enabled }}"},{"line_number":16,"context_line":"{{- $bootstrapJob :\u003d dict \"envAll\" . \"serviceName\" \"cinder\" \"keystoneUser\" .Values.bootstrap.ks_user \"logConfigFile\" .Values.conf.cinder.DEFAULT.log_config_append -}}"},{"line_number":17,"context_line":"{{- if and .Values.manifests.certificates (index .Values.endpoints.identity.auth .Values.bootstrap.ks_user \"cacert\") -}}"},{"line_number":18,"context_line":"{{- $_ :\u003d set $bootstrapJob \"tlsSecret\" .Values.secrets.tls.volume.api.internal -}}"},{"line_number":19,"context_line":"{{- end -}}"},{"line_number":20,"context_line":"{{ $bootstrapJob | include \"helm-toolkit.manifests.job_bootstrap\" }}"}],"source_content_type":"text/x-yaml","patch_set":12,"id":"9ee940e9_42af69cd","line":17,"range":{"start_line":17,"start_character":42,"end_line":17,"end_character":120},"in_reply_to":"3d0a8e63_c0e0e350","updated":"2021-03-08 15:25:31.000000000","message":"This is another null check, since the Keystone-related secret (https://review.opendev.org/plugins/gitiles/openstack/openstack-helm/+/refs/heads/master/cinder/templates/secret-keystone.yaml) is populated based on these lines:\n- `{{- $userContext :\u003d index $context.Values.endpoints.identity.auth $userClass }}` (https://review.opendev.org/plugins/gitiles/openstack/openstack-helm-infra/+/refs/heads/master/helm-toolkit/templates/snippets/_keystone_secret_openrc.tpl#19)\n- `{{- if $userContext.cacert }}` (https://review.opendev.org/plugins/gitiles/openstack/openstack-helm-infra/+/refs/heads/master/helm-toolkit/templates/snippets/_keystone_secret_openrc.tpl#29)\n\nThat secret is then used in this bootstrap job:\n- `{{- with $env :\u003d dict \"ksUserSecret\" ( index $envAll.Values.secrets.identity $keystoneUser ) \"useCA\" (ne $tlsSecret \"\") }}` (https://review.opendev.org/plugins/gitiles/openstack/openstack-helm-infra/+/refs/heads/master/helm-toolkit/templates/manifests/_job-bootstrap.tpl#82)\n- https://review.opendev.org/plugins/gitiles/openstack/openstack-helm-infra/+/refs/heads/master/helm-toolkit/templates/snippets/_keystone_openrc_env_vars.tpl#135\n\nEspecially `_job-bootstrap.tpl`\u0027s L82 condition for populating `OS_CACERT` relies on the secret *just* existing and not on the condition specified in `_keystone_secret_openrc.tpl`\u0027s L29 used to define the `OS_CACERT` field based on `cacert`\u0027s value.\n\nThis means that, before this check, the bootstrap job will *always* attempt to use `OS_CACERT` from the Keystone secret, regardless of whether that key is actually defined there.","commit_id":"48583bbaa4132d224fd8e94c612acdca315430f2"},{"author":{"_account_id":24780,"name":"Sangeet Gupta","email":"sg774j@att.com","username":"sgupta"},"change_message_id":"edf199ca64709232d03678546235c118b116c4c8","unresolved":true,"context_lines":[{"line_number":14,"context_line":""},{"line_number":15,"context_line":"{{- if and .Values.manifests.job_bootstrap .Values.bootstrap.enabled }}"},{"line_number":16,"context_line":"{{- $bootstrapJob :\u003d dict \"envAll\" . \"serviceName\" \"cinder\" \"keystoneUser\" .Values.bootstrap.ks_user \"logConfigFile\" .Values.conf.cinder.DEFAULT.log_config_append -}}"},{"line_number":17,"context_line":"{{- if and .Values.manifests.certificates (index .Values.endpoints.identity.auth .Values.bootstrap.ks_user \"cacert\") -}}"},{"line_number":18,"context_line":"{{- $_ :\u003d set $bootstrapJob \"tlsSecret\" .Values.secrets.tls.volume.api.internal -}}"},{"line_number":19,"context_line":"{{- end -}}"},{"line_number":20,"context_line":"{{ $bootstrapJob | include \"helm-toolkit.manifests.job_bootstrap\" }}"}],"source_content_type":"text/x-yaml","patch_set":12,"id":"684b56b5_6e346c12","line":17,"range":{"start_line":17,"start_character":42,"end_line":17,"end_character":120},"in_reply_to":"9ee940e9_42af69cd","updated":"2021-03-10 14:38:20.000000000","message":"Same response as previous comment.","commit_id":"48583bbaa4132d224fd8e94c612acdca315430f2"}]}