You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2022/01/25 11:15:50 UTC
[GitHub] [superset] ad-m opened a new pull request #18161: feat(helm): Add schema of values in Helm Chart
ad-m opened a new pull request #18161:
URL: https://github.com/apache/superset/pull/18161
### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
I added schema validation for `values.yaml` as designed in #18127. My goal was to add a basic schema and then gradually expand it. I attempt to use remote schemas wherever possible.
I can see that overwhelmingly required fields may appear now. Removing these requirements requires very careful verification in multiple parameters that the Helm Chart is prepared to render without the parameter, which I intend to do when we work on the rendering tests.
I think it's better to agree on the structure and notation of the schema and then relax when working with Helm Chart is less fragile (more tests added).
I am aware that installation on air-gapped environments (due to the need to download remote schematics) will be blocked. However, I would also leave that for the next PR to make this PR easier to review.
If I had the permission to create a new branch in the `apache / superset` repository, I would create this PR against the feature branch, because the primary goal is to get insights on the approach, and the space for development is significant. However, I believe that this code already provides some value and is functional.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
N/A
### TESTING INSTRUCTIONS
<!--- Required! What steps can be taken to manually verify the changes? -->
You might change `values.yaml` and execute `helm lint` to make sure that linting errors are raised.
For the purpose, I keep in that PR two commits to ensure that CI detect mismatch `values.yaml` to `values.schema.json` thanks to `.github/workflows/superset-helm-lint.yml`. If it turns out otherwise - I will update that PR.
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue: see #18127
- [ ] Required feature flags: NO
- [ ] Changes UI: NO
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351)): NO
- [ ] Migration is atomic, supports rollback & is backwards-compatible: N/A
- [ ] Confirm DB migration upgrade and downgrade tested: N/A
- [ ] Runtime estimates and downtime expectations provided: N/A
- [ ] Introduces new feature or API: NO
- [ ] Removes existing feature or API: NO
@wiktor2200 @mvoitko might be interested in that area as involved in development of our Helm Chart.
@mik-laj @potiuk @kaxil may want to share their thoughts based on their involvement in the discussion in #18127.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] ad-m commented on pull request #18161: feat(helm): Add schema of values in Helm Chart
Posted by GitBox <gi...@apache.org>.
ad-m commented on pull request #18161:
URL: https://github.com/apache/superset/pull/18161#issuecomment-1021086502
@craig-rueda could you take a look as maintainer of Chart?
https://github.com/apache/superset/blob/fa104fee9a20dbd2175f60c50043886b8b8f7408/helm/superset/Chart.yaml#L21-L24
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] ad-m commented on pull request #18161: feat(helm): Add schema of values in Helm Chart
Posted by GitBox <gi...@apache.org>.
ad-m commented on pull request #18161:
URL: https://github.com/apache/superset/pull/18161#issuecomment-1021086502
@craig-rueda could you take a look as maintainer of Chart?
https://github.com/apache/superset/blob/fa104fee9a20dbd2175f60c50043886b8b8f7408/helm/superset/Chart.yaml#L21-L24
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] ad-m commented on a change in pull request #18161: feat(helm): Add schema of values in Helm Chart
Posted by GitBox <gi...@apache.org>.
ad-m commented on a change in pull request #18161:
URL: https://github.com/apache/superset/pull/18161#discussion_r792139750
##########
File path: helm/superset/values.schema.json
##########
@@ -0,0 +1,590 @@
+{
+ "$schema": "http://json-schema.org/draft-04/schema#",
+ "type": "object",
+ "additionalProperties": false,
+ "properties": {
+ "replicaCount": {
+ "type": "integer"
+ },
+ "runAsUser": {
+ "type": "integer"
+ },
+ "serviceAccount": {
+ "type": "object",
+ "additionalProperties": false,
+ "properties": {
+ "create": {
+ "type": "boolean"
+ }
+ },
+ "required": [
+ "create"
+ ]
+ },
+ "bootstrapScript": {
+ "type": "string"
+ },
+ "configFromSecret": {
+ "type": "string"
+ },
+ "envFromSecret": {
+ "type": "string"
+ },
+ "envFromSecrets": {
+ "type": "array"
+ },
+ "extraEnv": {
+ "type": "object"
+ },
+ "extraEnvRaw": {
+ "type": "array"
+ },
+ "extraSecretEnv": {
+ "type": "object"
+ },
+ "extraConfigs": {
+ "type": "object"
+ },
+ "extraSecrets": {
+ "type": "object"
+ },
+ "extraVolumes": {
+ "$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.23.0/_definitions.json##/definitions/io.k8s.api.apps.v1.PodSpec/properties/volumes"
+ },
+ "extraVolumeMounts": {
+ "$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.23.0/_definitions.json##/definitions/io.k8s.api.apps.v1.PodSpec/properties/volumeMounts"
+ },
+ "configOverrides": {
+ "type": "object",
+ "additionalProperties": false,
+ "properties": {
+ "extend_timeout": {
+ "type": "string"
+ },
+ "enable_oauth": {
+ "type": "string"
+ }
+ }
+ },
+ "configOverridesFiles": {
+ "type": "object",
+ "additionalProperties": false,
+ "properties": {
+ "extend_timeout": {
+ "type": "string"
+ },
+ "enable_oauth": {
+ "type": "string"
+ }
+ }
+ },
+ "configMountPath": {
+ "type": "string"
+ },
+ "extraConfigMountPath": {
+ "type": "string"
+ },
+ "image": {
+ "type": "object",
+ "additionalProperties": false,
+ "properties": {
+ "repository": {
+ "type": "string"
+ },
+ "tag": {
+ "type": "string"
+ },
+ "pullPolicy": {
+ "$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.23.0/_definitions.json##/definitions/io.k8s.api.core.v1.Container/properties/imagePullPolicy"
+ }
+ },
+ "required": [
+ "repository",
+ "tag",
+ "pullPolicy"
+ ]
+ },
+ "imagePullSecrets": {
+ "type": "array"
+ },
+ "service": {
+ "type": "object",
+ "additionalProperties": false,
+ "properties": {
+ "type": {
+ "$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.23.0/_definitions.json##/definitions/io.k8s.api.core.v1.ServiceSpec/properties/type"
+ },
+ "port": {
+ "type": "integer"
+ },
+ "annotations": {
+ "$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.23.0/_definitions.json##/definitions/io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta/properties/annotations"
+ },
+ "loadBalancerIP": {
+ "$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.23.0/_definitions.json##/definitions/io.k8s.api.core.v1.ServiceSpec/properties/loadBalancerIP"
+ }
+ },
+ "required": [
+ "type",
+ "port"
+ ]
+ },
+ "ingress": {
+ "type": "object",
+ "additionalProperties": false,
+ "properties": {
+ "enabled": {
+ "type": "boolean"
+ },
+ "annotations": {
+ "$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.23.0/_definitions.json##/definitions/io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta/properties/annotations"
+ },
+ "path": {
+ "type": "string"
+ },
+ "pathType": {
+ "type": "string"
+ },
+ "hosts": {
+ "type": "array",
+ "items": {
+ "type": "string"
+ }
+ },
+ "tls": {
+ "type": "array",
+ "items": {
+ "type": "object",
+ "additionalProperties": false,
+ "properties": {
+ "secretName": {
+ "type": "string"
+ },
+ "hosts": {
+ "type": "array",
+ "items": {
+ "type": "string"
+ }
+ }
+ }
+ }
+ }
+ },
+ "required": [
+ "enabled",
+ "annotations",
+ "path",
+ "pathType",
+ "hosts",
+ "tls"
+ ]
+ },
+ "resources": {
+ "type": "object",
+ "additionalProperties": false,
+ "properties": {
Review comment:
Yes, I miss that one. Fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] craig-rueda commented on a change in pull request #18161: feat(helm): Add schema of values in Helm Chart
Posted by GitBox <gi...@apache.org>.
craig-rueda commented on a change in pull request #18161:
URL: https://github.com/apache/superset/pull/18161#discussion_r792052888
##########
File path: helm/superset/values.yaml
##########
@@ -357,7 +357,7 @@ postgresql:
enabled: true
##
## The name of an existing secret that contains the postgres password.
- existingSecret:
+ existingSecret: secret-name
Review comment:
Can we do something like `null` instead?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] mik-laj commented on a change in pull request #18161: feat(helm): Add schema of values in Helm Chart
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #18161:
URL: https://github.com/apache/superset/pull/18161#discussion_r792039501
##########
File path: helm/superset/values.schema.json
##########
@@ -0,0 +1,590 @@
+{
+ "$schema": "http://json-schema.org/draft-04/schema#",
+ "type": "object",
+ "additionalProperties": false,
+ "properties": {
+ "replicaCount": {
+ "type": "integer"
+ },
+ "runAsUser": {
+ "type": "integer"
+ },
+ "serviceAccount": {
+ "type": "object",
+ "additionalProperties": false,
+ "properties": {
+ "create": {
+ "type": "boolean"
+ }
+ },
+ "required": [
+ "create"
+ ]
+ },
+ "bootstrapScript": {
+ "type": "string"
+ },
+ "configFromSecret": {
+ "type": "string"
+ },
+ "envFromSecret": {
+ "type": "string"
+ },
+ "envFromSecrets": {
+ "type": "array"
+ },
+ "extraEnv": {
+ "type": "object"
+ },
+ "extraEnvRaw": {
+ "type": "array"
+ },
+ "extraSecretEnv": {
+ "type": "object"
+ },
+ "extraConfigs": {
+ "type": "object"
+ },
+ "extraSecrets": {
+ "type": "object"
+ },
+ "extraVolumes": {
+ "$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.23.0/_definitions.json##/definitions/io.k8s.api.apps.v1.PodSpec/properties/volumes"
+ },
+ "extraVolumeMounts": {
+ "$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.23.0/_definitions.json##/definitions/io.k8s.api.apps.v1.PodSpec/properties/volumeMounts"
+ },
+ "configOverrides": {
+ "type": "object",
+ "additionalProperties": false,
+ "properties": {
+ "extend_timeout": {
+ "type": "string"
+ },
+ "enable_oauth": {
+ "type": "string"
+ }
+ }
+ },
+ "configOverridesFiles": {
+ "type": "object",
+ "additionalProperties": false,
+ "properties": {
+ "extend_timeout": {
+ "type": "string"
+ },
+ "enable_oauth": {
+ "type": "string"
+ }
+ }
+ },
+ "configMountPath": {
+ "type": "string"
+ },
+ "extraConfigMountPath": {
+ "type": "string"
+ },
+ "image": {
+ "type": "object",
+ "additionalProperties": false,
+ "properties": {
+ "repository": {
+ "type": "string"
+ },
+ "tag": {
+ "type": "string"
+ },
+ "pullPolicy": {
+ "$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.23.0/_definitions.json##/definitions/io.k8s.api.core.v1.Container/properties/imagePullPolicy"
+ }
+ },
+ "required": [
+ "repository",
+ "tag",
+ "pullPolicy"
+ ]
+ },
+ "imagePullSecrets": {
+ "type": "array"
+ },
+ "service": {
+ "type": "object",
+ "additionalProperties": false,
+ "properties": {
+ "type": {
+ "$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.23.0/_definitions.json##/definitions/io.k8s.api.core.v1.ServiceSpec/properties/type"
+ },
+ "port": {
+ "type": "integer"
+ },
+ "annotations": {
+ "$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.23.0/_definitions.json##/definitions/io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta/properties/annotations"
+ },
+ "loadBalancerIP": {
+ "$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.23.0/_definitions.json##/definitions/io.k8s.api.core.v1.ServiceSpec/properties/loadBalancerIP"
+ }
+ },
+ "required": [
+ "type",
+ "port"
+ ]
+ },
+ "ingress": {
+ "type": "object",
+ "additionalProperties": false,
+ "properties": {
+ "enabled": {
+ "type": "boolean"
+ },
+ "annotations": {
+ "$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.23.0/_definitions.json##/definitions/io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta/properties/annotations"
+ },
+ "path": {
+ "type": "string"
+ },
+ "pathType": {
+ "type": "string"
+ },
+ "hosts": {
+ "type": "array",
+ "items": {
+ "type": "string"
+ }
+ },
+ "tls": {
+ "type": "array",
+ "items": {
+ "type": "object",
+ "additionalProperties": false,
+ "properties": {
+ "secretName": {
+ "type": "string"
+ },
+ "hosts": {
+ "type": "array",
+ "items": {
+ "type": "string"
+ }
+ }
+ }
+ }
+ }
+ },
+ "required": [
+ "enabled",
+ "annotations",
+ "path",
+ "pathType",
+ "hosts",
+ "tls"
+ ]
+ },
+ "resources": {
+ "type": "object",
+ "additionalProperties": false,
+ "properties": {
Review comment:
Can we use reference to k8s spec here?
https://github.com/apache/superset/blob/9900e5a6891929876b8fb7b5d29c25cd5b042626/helm/superset/templates/deployment.yaml#L115
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] ad-m commented on pull request #18161: feat(helm): Add schema of values in Helm Chart
Posted by GitBox <gi...@apache.org>.
ad-m commented on pull request #18161:
URL: https://github.com/apache/superset/pull/18161#issuecomment-1021664216
> Thanks for the submission! Can you add a `helm lint` call in CI as well? Also, I see a few things that were changed in the `values.yaml` which I think might cause some issues with existing template behavior.
We have already one: https://github.com/apache/superset/blob/2491b89f2911482d9951064e541d616e396f75eb/.github/workflows/superset-helm-lint.yml#L41-L45
I confirmed that `ct` lint `values.yaml` agains `values.schema.json` too:
```
$ CT_CHART_DIRS="helm" ct lint --print-config
Linting charts...
------------------------------------------------------------------------------------------------------------------------
Configuration
------------------------------------------------------------------------------------------------------------------------
Remote: origin
TargetBranch: master
Since: HEAD
BuildId:
LintConf: lintconf.yaml
ChartYamlSchema: /home/adas/.ct/chart_schema.yaml
ValidateMaintainers: true
ValidateChartSchema: true
ValidateYaml: true
AdditionalCommands: []
CheckVersionIncrement: true
ProcessAllCharts: false
Charts: []
ChartRepos: []
ChartDirs: [helm]
ExcludedCharts: []
HelmExtraArgs:
HelmRepoExtraArgs: []
Debug: false
Upgrade: false
SkipMissingValues: false
Namespace:
ReleaseLabel:
ExcludeDeprecated: false
------------------------------------------------------------------------------------------------------------------------
------------------------------------------------------------------------------------------------------------------------
Charts to be processed:
------------------------------------------------------------------------------------------------------------------------
superset => (version: "0.5.5", path: "helm/superset")
------------------------------------------------------------------------------------------------------------------------
Hang tight while we grab the latest from your chart repositories...
...Successfully got an update from the "bitnami" chart repository
Update Complete. ⎈Happy Helming!⎈
Saving 2 charts
Downloading postgresql from repo https://charts.bitnami.com/bitnami
Downloading redis from repo https://charts.bitnami.com/bitnami
Deleting outdated charts
Linting chart 'superset => (version: "0.5.5", path: "helm/superset")'
Checking chart 'superset => (version: "0.5.5", path: "helm/superset")' for a version bump...
Old chart version: 0.5.4
New chart version: 0.5.5
Chart version ok.
Validating /home/adas/Devel/superset/helm/superset/Chart.yaml...
Validation success! 👍
Validating maintainers...
==> Linting helm/superset
[INFO] Chart.yaml: icon is recommended
[ERROR] values.yaml: - supersetWorker.forceReload: Invalid type. Expected: boolean, given: array
[ERROR] templates/: values don't meet the specifications of the schema(s) in the following chart(s):
superset:
- supersetWorker.forceReload: Invalid type. Expected: boolean, given: array
Error: 1 chart(s) linted, 1 chart(s) failed
------------------------------------------------------------------------------------------------------------------------
✖︎ superset => (version: "0.5.5", path: "helm/superset") > Error waiting for process: exit status 1
------------------------------------------------------------------------------------------------------------------------
Error: Error linting charts: Error processing charts
Error linting charts: Error processing charts
```
I confirmed also that it successfully running on CI, so I wrote in the original pull request description:
> For the purpose, I keep in that PR two commits to ensure that CI detect mismatch `values.yaml` to `values.schema.json` thanks to `.github/workflows/superset-helm-lint.yml`. If it turns out otherwise - I will update that PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] craig-rueda commented on a change in pull request #18161: feat(helm): Add schema of values in Helm Chart
Posted by GitBox <gi...@apache.org>.
craig-rueda commented on a change in pull request #18161:
URL: https://github.com/apache/superset/pull/18161#discussion_r792051367
##########
File path: helm/superset/values.yaml
##########
@@ -357,7 +357,7 @@ postgresql:
enabled: true
##
## The name of an existing secret that contains the postgres password.
- existingSecret:
+ existingSecret: secret-name
Review comment:
These changes will/can cause unintended consequences.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] mik-laj commented on a change in pull request #18161: feat(helm): Add schema of values in Helm Chart
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #18161:
URL: https://github.com/apache/superset/pull/18161#discussion_r792146121
##########
File path: helm/superset/values.yaml
##########
@@ -357,11 +357,9 @@ postgresql:
enabled: true
##
## The name of an existing secret that contains the postgres password.
- existingSecret:
+
## Name of the key containing the secret.
- existingSecretKey: postgresql-password
- ##
- ## If you are bringing your own PostgreSQL, you should set postgresHost and
+ existingSecretKey: If you are bringing your own PostgreSQL, you should set postgresHost and
Review comment:
```suggestion
existingSecretKey:
##
## If you are bringing your own PostgreSQL, you should set postgresHost and
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] mik-laj commented on a change in pull request #18161: feat(helm): Add schema of values in Helm Chart
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #18161:
URL: https://github.com/apache/superset/pull/18161#discussion_r792036264
##########
File path: helm/superset/values.schema.json
##########
@@ -0,0 +1,590 @@
+{
+ "$schema": "http://json-schema.org/draft-04/schema#",
+ "type": "object",
+ "additionalProperties": false,
+ "properties": {
+ "replicaCount": {
+ "type": "integer"
+ },
+ "runAsUser": {
+ "type": "integer"
+ },
+ "serviceAccount": {
+ "type": "object",
+ "additionalProperties": false,
+ "properties": {
+ "create": {
+ "type": "boolean"
+ }
+ },
+ "required": [
+ "create"
+ ]
+ },
+ "bootstrapScript": {
+ "type": "string"
+ },
+ "configFromSecret": {
+ "type": "string"
+ },
+ "envFromSecret": {
+ "type": "string"
+ },
+ "envFromSecrets": {
+ "type": "array"
+ },
+ "extraEnv": {
+ "type": "object"
+ },
+ "extraEnvRaw": {
+ "type": "array"
+ },
+ "extraSecretEnv": {
+ "type": "object"
+ },
+ "extraConfigs": {
+ "type": "object"
+ },
+ "extraSecrets": {
+ "type": "object"
+ },
+ "extraVolumes": {
+ "$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.23.0/_definitions.json##/definitions/io.k8s.api.apps.v1.PodSpec/properties/volumes"
+ },
+ "extraVolumeMounts": {
+ "$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.23.0/_definitions.json##/definitions/io.k8s.api.apps.v1.PodSpec/properties/volumeMounts"
+ },
+ "configOverrides": {
+ "type": "object",
+ "additionalProperties": false,
+ "properties": {
+ "extend_timeout": {
+ "type": "string"
+ },
+ "enable_oauth": {
+ "type": "string"
+ }
+ }
Review comment:
```suggestion
"additionalProperties": true,
"properties": {
"type": "object",
"properties": {
"type": "string"
}
}
```
We support many configuration options, the set of possible values is determined by the superset version. To be future-proof, I propose to give us full freedom here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] craig-rueda merged pull request #18161: feat(helm): Add schema of values in Helm Chart
Posted by GitBox <gi...@apache.org>.
craig-rueda merged pull request #18161:
URL: https://github.com/apache/superset/pull/18161
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] ad-m commented on a change in pull request #18161: feat(helm): Add schema of values in Helm Chart
Posted by GitBox <gi...@apache.org>.
ad-m commented on a change in pull request #18161:
URL: https://github.com/apache/superset/pull/18161#discussion_r792139236
##########
File path: helm/superset/values.schema.json
##########
@@ -0,0 +1,590 @@
+{
+ "$schema": "http://json-schema.org/draft-04/schema#",
+ "type": "object",
+ "additionalProperties": false,
+ "properties": {
+ "replicaCount": {
+ "type": "integer"
+ },
+ "runAsUser": {
+ "type": "integer"
+ },
+ "serviceAccount": {
+ "type": "object",
+ "additionalProperties": false,
+ "properties": {
+ "create": {
+ "type": "boolean"
+ }
+ },
+ "required": [
+ "create"
+ ]
+ },
+ "bootstrapScript": {
+ "type": "string"
+ },
+ "configFromSecret": {
+ "type": "string"
+ },
+ "envFromSecret": {
+ "type": "string"
+ },
+ "envFromSecrets": {
+ "type": "array"
+ },
+ "extraEnv": {
+ "type": "object"
+ },
+ "extraEnvRaw": {
+ "type": "array"
+ },
+ "extraSecretEnv": {
+ "type": "object"
+ },
+ "extraConfigs": {
+ "type": "object"
+ },
+ "extraSecrets": {
+ "type": "object"
+ },
+ "extraVolumes": {
+ "$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.23.0/_definitions.json##/definitions/io.k8s.api.apps.v1.PodSpec/properties/volumes"
+ },
+ "extraVolumeMounts": {
+ "$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.23.0/_definitions.json##/definitions/io.k8s.api.apps.v1.PodSpec/properties/volumeMounts"
+ },
+ "configOverrides": {
+ "type": "object",
+ "additionalProperties": false,
+ "properties": {
+ "extend_timeout": {
+ "type": "string"
+ },
+ "enable_oauth": {
+ "type": "string"
+ }
+ }
Review comment:
I don't think that it gonna work in the way you will expect. You rather need:
```json
"additionalProperties": {
"type": "string"
},
```
In a moment I will commit that fix, so let me know what do you think.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] ad-m commented on a change in pull request #18161: feat(helm): Add schema of values in Helm Chart
Posted by GitBox <gi...@apache.org>.
ad-m commented on a change in pull request #18161:
URL: https://github.com/apache/superset/pull/18161#discussion_r792139975
##########
File path: helm/superset/values.yaml
##########
@@ -357,7 +357,7 @@ postgresql:
enabled: true
##
## The name of an existing secret that contains the postgres password.
- existingSecret:
+ existingSecret: secret-name
Review comment:
Yes, I a moment I will commit a fix for that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] ad-m commented on a change in pull request #18161: feat(helm): Add schema of values in Helm Chart
Posted by GitBox <gi...@apache.org>.
ad-m commented on a change in pull request #18161:
URL: https://github.com/apache/superset/pull/18161#discussion_r792139596
##########
File path: helm/superset/values.schema.json
##########
@@ -0,0 +1,590 @@
+{
+ "$schema": "http://json-schema.org/draft-04/schema#",
+ "type": "object",
+ "additionalProperties": false,
+ "properties": {
+ "replicaCount": {
+ "type": "integer"
+ },
+ "runAsUser": {
+ "type": "integer"
+ },
+ "serviceAccount": {
+ "type": "object",
+ "additionalProperties": false,
+ "properties": {
+ "create": {
+ "type": "boolean"
+ }
+ },
+ "required": [
+ "create"
+ ]
+ },
+ "bootstrapScript": {
+ "type": "string"
+ },
+ "configFromSecret": {
+ "type": "string"
+ },
+ "envFromSecret": {
+ "type": "string"
+ },
+ "envFromSecrets": {
+ "type": "array"
+ },
+ "extraEnv": {
+ "type": "object"
+ },
+ "extraEnvRaw": {
+ "type": "array"
+ },
+ "extraSecretEnv": {
+ "type": "object"
+ },
+ "extraConfigs": {
+ "type": "object"
+ },
+ "extraSecrets": {
+ "type": "object"
+ },
+ "extraVolumes": {
+ "$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.23.0/_definitions.json##/definitions/io.k8s.api.apps.v1.PodSpec/properties/volumes"
+ },
+ "extraVolumeMounts": {
+ "$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.23.0/_definitions.json##/definitions/io.k8s.api.apps.v1.PodSpec/properties/volumeMounts"
+ },
+ "configOverrides": {
+ "type": "object",
+ "additionalProperties": false,
+ "properties": {
+ "extend_timeout": {
+ "type": "string"
+ },
+ "enable_oauth": {
+ "type": "string"
+ }
+ }
Review comment:
Overall I fully agree that `additionalProperties` should be enabled there.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] craig-rueda commented on pull request #18161: feat(helm): Add schema of values in Helm Chart
Posted by GitBox <gi...@apache.org>.
craig-rueda commented on pull request #18161:
URL: https://github.com/apache/superset/pull/18161#issuecomment-1021536740
Thanks for the submission! Can you add a `helm lint` call in CI as well? Also, I see a few things that were changed in the `values.yaml` which I think might cause some issues with existing template behavior.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org