You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2021/10/23 01:08:45 UTC
[GitHub] [airflow] mik-laj opened a new pull request #19181: More strict schema for values.yaml
mik-laj opened a new pull request #19181:
URL: https://github.com/apache/airflow/pull/19181
Some fields in values.yml accept value that complies with the Kubernetes API, so we can verify that too.
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/main/UPDATING.md).
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #19181: More strict schema for values.yaml
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #19181:
URL: https://github.com/apache/airflow/pull/19181#issuecomment-959419084
The PR is likely OK to be merged with just subset of tests for default Python and Database versions without running the full matrix of tests, because it does not modify the core of Airflow. If the committers decide that the full tests matrix is needed, they will add the label 'full tests needed'. Then you should rebase to the latest main or amend the last commit of the PR, and push it with --force-with-lease.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jedcunningham commented on a change in pull request #19181: More strict schema for values.yaml
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on a change in pull request #19181:
URL: https://github.com/apache/airflow/pull/19181#discussion_r742029179
##########
File path: chart/tests/test_flower.py
##########
@@ -358,18 +358,18 @@ def test_defaults(self):
"ports, expected_ports",
[
(
- [{"name": "{{ .Release.Name }}", "protocol": "UDP", "port": "{{ .Values.ports.flowerUI }}"}],
- [{"name": "RELEASE-NAME", "protocol": "UDP", "port": 5555}],
+ [{"protocol": "UDP", "port": "{{ .Values.ports.flowerUI }}"}],
+ [{"protocol": "UDP", "port": 5555}],
),
- ([{"name": "only_sidecar", "port": "sidecar"}], [{"name": "only_sidecar", "port": "sidecar"}]),
+ ([{"port": "sidecar"}], [{"port": "sidecar"}]),
(
[
- {"name": "flower-ui", "port": "{{ .Values.ports.flowerUI }}"},
- {"name": "sidecar", "port": 80},
+ {"port": "{{ .Values.ports.flowerUI }}"},
+ {"port": 80},
],
[
- {"name": "flower-ui", "port": 5555},
- {"name": "sidecar", "port": 80},
+ {"port": 5555},
+ {"port": 80},
Review comment:
```suggestion
{"name": "flower-ui", "port": 5555},
{"name": "sidecar-RELEASE-NAME", "port": 80},
```
##########
File path: chart/tests/test_flower.py
##########
@@ -358,18 +358,18 @@ def test_defaults(self):
"ports, expected_ports",
[
(
- [{"name": "{{ .Release.Name }}", "protocol": "UDP", "port": "{{ .Values.ports.flowerUI }}"}],
- [{"name": "RELEASE-NAME", "protocol": "UDP", "port": 5555}],
+ [{"protocol": "UDP", "port": "{{ .Values.ports.flowerUI }}"}],
+ [{"protocol": "UDP", "port": 5555}],
),
- ([{"name": "only_sidecar", "port": "sidecar"}], [{"name": "only_sidecar", "port": "sidecar"}]),
+ ([{"port": "sidecar"}], [{"port": "sidecar"}]),
(
[
- {"name": "flower-ui", "port": "{{ .Values.ports.flowerUI }}"},
- {"name": "sidecar", "port": 80},
+ {"port": "{{ .Values.ports.flowerUI }}"},
+ {"port": 80},
Review comment:
```suggestion
{"name": "flower-ui", "port": "{{ .Values.ports.flowerUI }}"},
{"name": "sidecar-{{ .Release.Name }}", "port": 80},
```
`name` is required if there is more than 1 ServicePort (sorry, deepest I can link you): https://kubernetes.io/docs/reference/kubernetes-api/service-resources/service-v1/#ServiceSpec
Also add back a test that `name` is templated.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on pull request #19181: More strict schema for values.yaml
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #19181:
URL: https://github.com/apache/airflow/pull/19181#issuecomment-962613889
I deleted all added `additionalProperties` fields. It should be safe now.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #19181: More strict schema for values.yaml
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #19181:
URL: https://github.com/apache/airflow/pull/19181#discussion_r742133261
##########
File path: chart/tests/test_flower.py
##########
@@ -358,18 +358,18 @@ def test_defaults(self):
"ports, expected_ports",
[
(
- [{"name": "{{ .Release.Name }}", "protocol": "UDP", "port": "{{ .Values.ports.flowerUI }}"}],
- [{"name": "RELEASE-NAME", "protocol": "UDP", "port": 5555}],
+ [{"protocol": "UDP", "port": "{{ .Values.ports.flowerUI }}"}],
+ [{"protocol": "UDP", "port": 5555}],
),
- ([{"name": "only_sidecar", "port": "sidecar"}], [{"name": "only_sidecar", "port": "sidecar"}]),
+ ([{"port": "sidecar"}], [{"port": "sidecar"}]),
(
[
- {"name": "flower-ui", "port": "{{ .Values.ports.flowerUI }}"},
- {"name": "sidecar", "port": 80},
+ {"port": "{{ .Values.ports.flowerUI }}"},
+ {"port": 80},
Review comment:
This is a test for NetworkPolicy, not Service. In NetworkPolicy, we use [NetworkPolicyPort](https://github.com/yannh/kubernetes-json-schema/blob/master/v1.21.0/serviceport-v1.json), not [ServicePort](https://github.com/yannh/kubernetes-json-schema/blob/master/v1.21.0/serviceport-v1.json), so `name` field is illegal 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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jedcunningham commented on a change in pull request #19181: More strict schema for values.yaml
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on a change in pull request #19181:
URL: https://github.com/apache/airflow/pull/19181#discussion_r742155713
##########
File path: chart/tests/test_flower.py
##########
@@ -358,18 +358,18 @@ def test_defaults(self):
"ports, expected_ports",
[
(
- [{"name": "{{ .Release.Name }}", "protocol": "UDP", "port": "{{ .Values.ports.flowerUI }}"}],
- [{"name": "RELEASE-NAME", "protocol": "UDP", "port": 5555}],
+ [{"protocol": "UDP", "port": "{{ .Values.ports.flowerUI }}"}],
+ [{"protocol": "UDP", "port": 5555}],
),
- ([{"name": "only_sidecar", "port": "sidecar"}], [{"name": "only_sidecar", "port": "sidecar"}]),
+ ([{"port": "sidecar"}], [{"port": "sidecar"}]),
(
[
- {"name": "flower-ui", "port": "{{ .Values.ports.flowerUI }}"},
- {"name": "sidecar", "port": 80},
+ {"port": "{{ .Values.ports.flowerUI }}"},
+ {"port": 80},
Review comment:
Ah, whoops!
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jedcunningham commented on a change in pull request #19181: More strict schema for values.yaml
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on a change in pull request #19181:
URL: https://github.com/apache/airflow/pull/19181#discussion_r742021449
##########
File path: chart/values.schema.json
##########
@@ -2069,11 +2258,38 @@
"annotations": {
"description": "Annotations for the webserver Service.",
"type": "object",
- "default": {}
+ "default": {},
+ "additionalProperties": {
+ "type": "string"
+ }
},
"ports": {
"description": "Ports for the webserver Service.",
"type": "array",
+ "items": {
+ "type": "object",
+ "additionalProperties": false,
+ "properties": {
+ "name": {
+ "type": "string"
+ },
+ "port": {
+ "type": [
+ "string",
+ "integer"
+ ]
+ },
+ "targetPort": {
+ "type": [
+ "string",
+ "integer"
+ ]
+ },
+ "protocol": {
+ "type": "string"
+ }
+ }
+ },
Review comment:
Ah, good catch.
##########
File path: chart/tests/test_flower.py
##########
@@ -358,18 +358,18 @@ def test_defaults(self):
"ports, expected_ports",
[
(
- [{"name": "{{ .Release.Name }}", "protocol": "UDP", "port": "{{ .Values.ports.flowerUI }}"}],
- [{"name": "RELEASE-NAME", "protocol": "UDP", "port": 5555}],
+ [{"protocol": "UDP", "port": "{{ .Values.ports.flowerUI }}"}],
+ [{"protocol": "UDP", "port": 5555}],
),
- ([{"name": "only_sidecar", "port": "sidecar"}], [{"name": "only_sidecar", "port": "sidecar"}]),
+ ([{"port": "sidecar"}], [{"port": "sidecar"}]),
(
[
- {"name": "flower-ui", "port": "{{ .Values.ports.flowerUI }}"},
- {"name": "sidecar", "port": 80},
+ {"port": "{{ .Values.ports.flowerUI }}"},
+ {"port": 80},
],
[
- {"name": "flower-ui", "port": 5555},
- {"name": "sidecar", "port": 80},
+ {"port": 5555},
+ {"port": 80},
Review comment:
```suggestion
{"name": "flower-ui", "port": 5555},
{"name": "sidecar-RELEASE-NAME", "port": 80},
```
##########
File path: chart/tests/test_flower.py
##########
@@ -358,18 +358,18 @@ def test_defaults(self):
"ports, expected_ports",
[
(
- [{"name": "{{ .Release.Name }}", "protocol": "UDP", "port": "{{ .Values.ports.flowerUI }}"}],
- [{"name": "RELEASE-NAME", "protocol": "UDP", "port": 5555}],
+ [{"protocol": "UDP", "port": "{{ .Values.ports.flowerUI }}"}],
+ [{"protocol": "UDP", "port": 5555}],
),
- ([{"name": "only_sidecar", "port": "sidecar"}], [{"name": "only_sidecar", "port": "sidecar"}]),
+ ([{"port": "sidecar"}], [{"port": "sidecar"}]),
(
[
- {"name": "flower-ui", "port": "{{ .Values.ports.flowerUI }}"},
- {"name": "sidecar", "port": 80},
+ {"port": "{{ .Values.ports.flowerUI }}"},
+ {"port": 80},
Review comment:
```suggestion
{"name": "flower-ui", "port": "{{ .Values.ports.flowerUI }}"},
{"name": "sidecar-{{ .Release.Name }}", "port": 80},
```
`name` is required if there is more than 1 ServicePort (sorry, deepest I can link you): https://kubernetes.io/docs/reference/kubernetes-api/service-resources/service-v1/#ServiceSpec
Also add back a test that `name` is templated.
##########
File path: chart/tests/test_flower.py
##########
@@ -358,18 +358,18 @@ def test_defaults(self):
"ports, expected_ports",
[
(
- [{"name": "{{ .Release.Name }}", "protocol": "UDP", "port": "{{ .Values.ports.flowerUI }}"}],
- [{"name": "RELEASE-NAME", "protocol": "UDP", "port": 5555}],
+ [{"protocol": "UDP", "port": "{{ .Values.ports.flowerUI }}"}],
+ [{"protocol": "UDP", "port": 5555}],
),
- ([{"name": "only_sidecar", "port": "sidecar"}], [{"name": "only_sidecar", "port": "sidecar"}]),
+ ([{"port": "sidecar"}], [{"port": "sidecar"}]),
(
[
- {"name": "flower-ui", "port": "{{ .Values.ports.flowerUI }}"},
- {"name": "sidecar", "port": 80},
+ {"port": "{{ .Values.ports.flowerUI }}"},
+ {"port": 80},
Review comment:
Ah, whoops!
##########
File path: chart/values.schema.json
##########
@@ -2069,11 +2258,38 @@
"annotations": {
"description": "Annotations for the webserver Service.",
"type": "object",
- "default": {}
+ "default": {},
+ "additionalProperties": {
+ "type": "string"
+ }
},
"ports": {
"description": "Ports for the webserver Service.",
"type": "array",
+ "items": {
+ "type": "object",
+ "additionalProperties": false,
+ "properties": {
+ "name": {
+ "type": "string"
+ },
+ "port": {
+ "type": [
+ "string",
+ "integer"
+ ]
+ },
+ "targetPort": {
+ "type": [
+ "string",
+ "integer"
+ ]
+ },
+ "protocol": {
+ "type": "string"
+ }
+ }
+ },
Review comment:
Ah, good catch.
##########
File path: chart/tests/test_flower.py
##########
@@ -358,18 +358,18 @@ def test_defaults(self):
"ports, expected_ports",
[
(
- [{"name": "{{ .Release.Name }}", "protocol": "UDP", "port": "{{ .Values.ports.flowerUI }}"}],
- [{"name": "RELEASE-NAME", "protocol": "UDP", "port": 5555}],
+ [{"protocol": "UDP", "port": "{{ .Values.ports.flowerUI }}"}],
+ [{"protocol": "UDP", "port": 5555}],
),
- ([{"name": "only_sidecar", "port": "sidecar"}], [{"name": "only_sidecar", "port": "sidecar"}]),
+ ([{"port": "sidecar"}], [{"port": "sidecar"}]),
(
[
- {"name": "flower-ui", "port": "{{ .Values.ports.flowerUI }}"},
- {"name": "sidecar", "port": 80},
+ {"port": "{{ .Values.ports.flowerUI }}"},
+ {"port": 80},
],
[
- {"name": "flower-ui", "port": 5555},
- {"name": "sidecar", "port": 80},
+ {"port": 5555},
+ {"port": 80},
Review comment:
```suggestion
{"name": "flower-ui", "port": 5555},
{"name": "sidecar-RELEASE-NAME", "port": 80},
```
##########
File path: chart/tests/test_flower.py
##########
@@ -358,18 +358,18 @@ def test_defaults(self):
"ports, expected_ports",
[
(
- [{"name": "{{ .Release.Name }}", "protocol": "UDP", "port": "{{ .Values.ports.flowerUI }}"}],
- [{"name": "RELEASE-NAME", "protocol": "UDP", "port": 5555}],
+ [{"protocol": "UDP", "port": "{{ .Values.ports.flowerUI }}"}],
+ [{"protocol": "UDP", "port": 5555}],
),
- ([{"name": "only_sidecar", "port": "sidecar"}], [{"name": "only_sidecar", "port": "sidecar"}]),
+ ([{"port": "sidecar"}], [{"port": "sidecar"}]),
(
[
- {"name": "flower-ui", "port": "{{ .Values.ports.flowerUI }}"},
- {"name": "sidecar", "port": 80},
+ {"port": "{{ .Values.ports.flowerUI }}"},
+ {"port": 80},
Review comment:
```suggestion
{"name": "flower-ui", "port": "{{ .Values.ports.flowerUI }}"},
{"name": "sidecar-{{ .Release.Name }}", "port": 80},
```
`name` is required if there is more than 1 ServicePort (sorry, deepest I can link you): https://kubernetes.io/docs/reference/kubernetes-api/service-resources/service-v1/#ServiceSpec
Also add back a test that `name` is templated.
##########
File path: chart/tests/test_flower.py
##########
@@ -358,18 +358,18 @@ def test_defaults(self):
"ports, expected_ports",
[
(
- [{"name": "{{ .Release.Name }}", "protocol": "UDP", "port": "{{ .Values.ports.flowerUI }}"}],
- [{"name": "RELEASE-NAME", "protocol": "UDP", "port": 5555}],
+ [{"protocol": "UDP", "port": "{{ .Values.ports.flowerUI }}"}],
+ [{"protocol": "UDP", "port": 5555}],
),
- ([{"name": "only_sidecar", "port": "sidecar"}], [{"name": "only_sidecar", "port": "sidecar"}]),
+ ([{"port": "sidecar"}], [{"port": "sidecar"}]),
(
[
- {"name": "flower-ui", "port": "{{ .Values.ports.flowerUI }}"},
- {"name": "sidecar", "port": 80},
+ {"port": "{{ .Values.ports.flowerUI }}"},
+ {"port": 80},
Review comment:
Ah, whoops!
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #19181: More strict schema for values.yaml
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #19181:
URL: https://github.com/apache/airflow/pull/19181#discussion_r742133261
##########
File path: chart/tests/test_flower.py
##########
@@ -358,18 +358,18 @@ def test_defaults(self):
"ports, expected_ports",
[
(
- [{"name": "{{ .Release.Name }}", "protocol": "UDP", "port": "{{ .Values.ports.flowerUI }}"}],
- [{"name": "RELEASE-NAME", "protocol": "UDP", "port": 5555}],
+ [{"protocol": "UDP", "port": "{{ .Values.ports.flowerUI }}"}],
+ [{"protocol": "UDP", "port": 5555}],
),
- ([{"name": "only_sidecar", "port": "sidecar"}], [{"name": "only_sidecar", "port": "sidecar"}]),
+ ([{"port": "sidecar"}], [{"port": "sidecar"}]),
(
[
- {"name": "flower-ui", "port": "{{ .Values.ports.flowerUI }}"},
- {"name": "sidecar", "port": 80},
+ {"port": "{{ .Values.ports.flowerUI }}"},
+ {"port": 80},
Review comment:
This is a test for NetworkPolicy, not Service. In NetworkPolicy, we use [https://github.com/yannh/kubernetes-json-schema/blob/master/v1.21.0/networkpolicyport-networking-v1.json](NetworkPolicyPort), not [ServicePort](https://github.com/yannh/kubernetes-json-schema/blob/master/v1.21.0/serviceport-v1.json), so `name` field is illegal 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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jedcunningham commented on a change in pull request #19181: More strict schema for values.yaml
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on a change in pull request #19181:
URL: https://github.com/apache/airflow/pull/19181#discussion_r742021449
##########
File path: chart/values.schema.json
##########
@@ -2069,11 +2258,38 @@
"annotations": {
"description": "Annotations for the webserver Service.",
"type": "object",
- "default": {}
+ "default": {},
+ "additionalProperties": {
+ "type": "string"
+ }
},
"ports": {
"description": "Ports for the webserver Service.",
"type": "array",
+ "items": {
+ "type": "object",
+ "additionalProperties": false,
+ "properties": {
+ "name": {
+ "type": "string"
+ },
+ "port": {
+ "type": [
+ "string",
+ "integer"
+ ]
+ },
+ "targetPort": {
+ "type": [
+ "string",
+ "integer"
+ ]
+ },
+ "protocol": {
+ "type": "string"
+ }
+ }
+ },
Review comment:
Ah, good catch.
##########
File path: chart/tests/test_flower.py
##########
@@ -358,18 +358,18 @@ def test_defaults(self):
"ports, expected_ports",
[
(
- [{"name": "{{ .Release.Name }}", "protocol": "UDP", "port": "{{ .Values.ports.flowerUI }}"}],
- [{"name": "RELEASE-NAME", "protocol": "UDP", "port": 5555}],
+ [{"protocol": "UDP", "port": "{{ .Values.ports.flowerUI }}"}],
+ [{"protocol": "UDP", "port": 5555}],
),
- ([{"name": "only_sidecar", "port": "sidecar"}], [{"name": "only_sidecar", "port": "sidecar"}]),
+ ([{"port": "sidecar"}], [{"port": "sidecar"}]),
(
[
- {"name": "flower-ui", "port": "{{ .Values.ports.flowerUI }}"},
- {"name": "sidecar", "port": 80},
+ {"port": "{{ .Values.ports.flowerUI }}"},
+ {"port": 80},
],
[
- {"name": "flower-ui", "port": 5555},
- {"name": "sidecar", "port": 80},
+ {"port": 5555},
+ {"port": 80},
Review comment:
```suggestion
{"name": "flower-ui", "port": 5555},
{"name": "sidecar-RELEASE-NAME", "port": 80},
```
##########
File path: chart/tests/test_flower.py
##########
@@ -358,18 +358,18 @@ def test_defaults(self):
"ports, expected_ports",
[
(
- [{"name": "{{ .Release.Name }}", "protocol": "UDP", "port": "{{ .Values.ports.flowerUI }}"}],
- [{"name": "RELEASE-NAME", "protocol": "UDP", "port": 5555}],
+ [{"protocol": "UDP", "port": "{{ .Values.ports.flowerUI }}"}],
+ [{"protocol": "UDP", "port": 5555}],
),
- ([{"name": "only_sidecar", "port": "sidecar"}], [{"name": "only_sidecar", "port": "sidecar"}]),
+ ([{"port": "sidecar"}], [{"port": "sidecar"}]),
(
[
- {"name": "flower-ui", "port": "{{ .Values.ports.flowerUI }}"},
- {"name": "sidecar", "port": 80},
+ {"port": "{{ .Values.ports.flowerUI }}"},
+ {"port": 80},
Review comment:
```suggestion
{"name": "flower-ui", "port": "{{ .Values.ports.flowerUI }}"},
{"name": "sidecar-{{ .Release.Name }}", "port": 80},
```
`name` is required if there is more than 1 ServicePort (sorry, deepest I can link you): https://kubernetes.io/docs/reference/kubernetes-api/service-resources/service-v1/#ServiceSpec
Also add back a test that `name` is templated.
##########
File path: chart/tests/test_flower.py
##########
@@ -358,18 +358,18 @@ def test_defaults(self):
"ports, expected_ports",
[
(
- [{"name": "{{ .Release.Name }}", "protocol": "UDP", "port": "{{ .Values.ports.flowerUI }}"}],
- [{"name": "RELEASE-NAME", "protocol": "UDP", "port": 5555}],
+ [{"protocol": "UDP", "port": "{{ .Values.ports.flowerUI }}"}],
+ [{"protocol": "UDP", "port": 5555}],
),
- ([{"name": "only_sidecar", "port": "sidecar"}], [{"name": "only_sidecar", "port": "sidecar"}]),
+ ([{"port": "sidecar"}], [{"port": "sidecar"}]),
(
[
- {"name": "flower-ui", "port": "{{ .Values.ports.flowerUI }}"},
- {"name": "sidecar", "port": 80},
+ {"port": "{{ .Values.ports.flowerUI }}"},
+ {"port": 80},
Review comment:
Ah, whoops!
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #19181: More strict schema for values.yaml
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #19181:
URL: https://github.com/apache/airflow/pull/19181#discussion_r741610474
##########
File path: chart/values.schema.json
##########
@@ -89,13 +89,18 @@
"description": "Specify scheduling constraints for all pods.",
"type": "object",
"default": {},
- "x-docsSection": "Kubernetes"
+ "x-docsSection": "Kubernetes",
+ "$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0/_definitions.json#/definitions/io.k8s.api.core.v1.Affinity"
},
"tolerations": {
"description": "Specify Tolerations for all pods.",
"type": "array",
"default": [],
- "x-docsSection": "Kubernetes"
+ "x-docsSection": "Kubernetes",
+ "items": {
+ "type": "object",
+ "$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0/_definitions.json#/definitions/io.k8s.api.core.v1.Toleration"
Review comment:
Fie `_definitions.json` is easier to browse as we can use CTRL + F to search by field name.
##########
File path: chart/values.schema.json
##########
@@ -2069,11 +2258,38 @@
"annotations": {
"description": "Annotations for the webserver Service.",
"type": "object",
- "default": {}
+ "default": {},
+ "additionalProperties": {
+ "type": "string"
+ }
},
"ports": {
"description": "Ports for the webserver Service.",
"type": "array",
+ "items": {
+ "type": "object",
+ "additionalProperties": false,
+ "properties": {
+ "name": {
+ "type": "string"
+ },
+ "port": {
+ "type": [
+ "string",
+ "integer"
+ ]
+ },
+ "targetPort": {
+ "type": [
+ "string",
+ "integer"
+ ]
+ },
+ "protocol": {
+ "type": "string"
+ }
+ }
+ },
Review comment:
No. It is not compatible with current default value:
```
{
"name": "airflow-ui",
"port": "{{ .Values.ports.airflowUI }}"
}
```
In the Kubernetes schema, the `port` field is defined as `integer`, but we use `string`.
##########
File path: chart/values.schema.json
##########
@@ -568,6 +601,23 @@
"type": "array",
"x-docsSection": "Airflow",
"default": [],
+ "items": {
+ "type": "object",
+ "properties": {
+ "envName": {
+ "description": "The name of the environment variable under which the secret will be available",
+ "type": "string"
+ },
+ "secretName": {
+ "description": "The name of the Kubernetes secret that will be read",
+ "type": "string"
+ },
+ "secretKey": {
+ "description": "The key of the Kubernetes secret",
+ "type": "string"
+ }
+ }
Review comment:
Thanks. I also checked and ` "additionalProperties": false,` was missing from the other definitions as well.
##########
File path: chart/values.schema.json
##########
@@ -89,13 +89,18 @@
"description": "Specify scheduling constraints for all pods.",
"type": "object",
"default": {},
- "x-docsSection": "Kubernetes"
+ "x-docsSection": "Kubernetes",
+ "$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0/_definitions.json#/definitions/io.k8s.api.core.v1.Affinity"
},
"tolerations": {
"description": "Specify Tolerations for all pods.",
"type": "array",
"default": [],
- "x-docsSection": "Kubernetes"
+ "x-docsSection": "Kubernetes",
+ "items": {
+ "type": "object",
+ "$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0/_definitions.json#/definitions/io.k8s.api.core.v1.Toleration"
Review comment:
Good idea. Updated. I added `"additionalProperties": false,` in a few place also.
##########
File path: chart/values.schema.json
##########
@@ -89,13 +89,18 @@
"description": "Specify scheduling constraints for all pods.",
"type": "object",
"default": {},
- "x-docsSection": "Kubernetes"
+ "x-docsSection": "Kubernetes",
+ "$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0/_definitions.json#/definitions/io.k8s.api.core.v1.Affinity"
},
"tolerations": {
"description": "Specify Tolerations for all pods.",
"type": "array",
"default": [],
- "x-docsSection": "Kubernetes"
+ "x-docsSection": "Kubernetes",
+ "items": {
+ "type": "object",
+ "$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0/_definitions.json#/definitions/io.k8s.api.core.v1.Toleration"
Review comment:
I checked, and we use a very narrow subset of object types, so it's better to fetch each definition separately.
```
cat chart/values.schema.json | jq '.. | objects| .["$ref"] | strings' | sort | uniq
"https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0-standalone-strict/_definitions.json#/definitions/io.k8s.api.apps.v1.DeploymentStrategy"
"https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0-standalone-strict/_definitions.json#/definitions/io.k8s.api.core.v1.Affinity"
"https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0-standalone-strict/_definitions.json#/definitions/io.k8s.api.core.v1.Container"
"https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0-standalone-strict/_definitions.json#/definitions/io.k8s.api.core.v1.HostAlias"
"https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0-standalone-strict/_definitions.json#/definitions/io.k8s.api.core.v1.LimitRangeItem"
"https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0-standalone-strict/_definitions.json#/definitions/io.k8s.api.core.v1.ResourceRequirements"
"https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0-standalone-strict/_definitions.json#/definitions/io.k8s.api.core.v1.Toleration"
"https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0-standalone-strict/_definitions.json#/definitions/io.k8s.api.core.v1.Volume"
"https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0-standalone-strict/_definitions.json#/definitions/io.k8s.api.core.v1.VolumeMount"
"https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0-standalone-strict/_definitions.json#/definitions/io.k8s.api.networking.v1.NetworkPolicyPeer"
"https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0-standalone-strict/_definitions.json#/definitions/io.k8s.api.networking.v1.NetworkPolicyPort"
"https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0-standalone-strict/_definitions.json#/definitions/io.k8s.apimachinery.pkg.api.resource.Quantity"
```
##########
File path: chart/tests/test_flower.py
##########
@@ -358,18 +358,18 @@ def test_defaults(self):
"ports, expected_ports",
[
(
- [{"name": "{{ .Release.Name }}", "protocol": "UDP", "port": "{{ .Values.ports.flowerUI }}"}],
- [{"name": "RELEASE-NAME", "protocol": "UDP", "port": 5555}],
+ [{"protocol": "UDP", "port": "{{ .Values.ports.flowerUI }}"}],
+ [{"protocol": "UDP", "port": 5555}],
),
- ([{"name": "only_sidecar", "port": "sidecar"}], [{"name": "only_sidecar", "port": "sidecar"}]),
+ ([{"port": "sidecar"}], [{"port": "sidecar"}]),
(
[
- {"name": "flower-ui", "port": "{{ .Values.ports.flowerUI }}"},
- {"name": "sidecar", "port": 80},
+ {"port": "{{ .Values.ports.flowerUI }}"},
+ {"port": 80},
Review comment:
This is a test for NetworkPolicy, not Service. In NetworkPolicy, we use [https://github.com/yannh/kubernetes-json-schema/blob/master/v1.21.0/networkpolicyport-networking-v1.json](NetworkPolicyPort), not [ServicePort](https://github.com/yannh/kubernetes-json-schema/blob/master/v1.21.0/serviceport-v1.json), so `name` field is illegal here.
##########
File path: chart/tests/test_flower.py
##########
@@ -358,18 +358,18 @@ def test_defaults(self):
"ports, expected_ports",
[
(
- [{"name": "{{ .Release.Name }}", "protocol": "UDP", "port": "{{ .Values.ports.flowerUI }}"}],
- [{"name": "RELEASE-NAME", "protocol": "UDP", "port": 5555}],
+ [{"protocol": "UDP", "port": "{{ .Values.ports.flowerUI }}"}],
+ [{"protocol": "UDP", "port": 5555}],
),
- ([{"name": "only_sidecar", "port": "sidecar"}], [{"name": "only_sidecar", "port": "sidecar"}]),
+ ([{"port": "sidecar"}], [{"port": "sidecar"}]),
(
[
- {"name": "flower-ui", "port": "{{ .Values.ports.flowerUI }}"},
- {"name": "sidecar", "port": 80},
+ {"port": "{{ .Values.ports.flowerUI }}"},
+ {"port": 80},
Review comment:
This is a test for NetworkPolicy, not Service. In NetworkPolicy, we use [NetworkPolicyPort](https://github.com/yannh/kubernetes-json-schema/blob/master/v1.21.0/serviceport-v1.json), not [ServicePort](https://github.com/yannh/kubernetes-json-schema/blob/master/v1.21.0/serviceport-v1.json), so `name` field is illegal here.
##########
File path: chart/tests/test_flower.py
##########
@@ -358,18 +358,18 @@ def test_defaults(self):
"ports, expected_ports",
[
(
- [{"name": "{{ .Release.Name }}", "protocol": "UDP", "port": "{{ .Values.ports.flowerUI }}"}],
- [{"name": "RELEASE-NAME", "protocol": "UDP", "port": 5555}],
+ [{"protocol": "UDP", "port": "{{ .Values.ports.flowerUI }}"}],
+ [{"protocol": "UDP", "port": 5555}],
),
- ([{"name": "only_sidecar", "port": "sidecar"}], [{"name": "only_sidecar", "port": "sidecar"}]),
+ ([{"port": "sidecar"}], [{"port": "sidecar"}]),
(
[
- {"name": "flower-ui", "port": "{{ .Values.ports.flowerUI }}"},
- {"name": "sidecar", "port": 80},
+ {"port": "{{ .Values.ports.flowerUI }}"},
+ {"port": 80},
Review comment:
This is a test for NetworkPolicy, not Service. In NetworkPolicy, we use [NetworkPolicyPort](https://github.com/yannh/kubernetes-json-schema/blob/master/v1.21.0/networkpolicyport-networking-v1.json), not [ServicePort](https://github.com/yannh/kubernetes-json-schema/blob/master/v1.21.0/serviceport-v1.json), so `name` field is illegal 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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on pull request #19181: More strict schema for values.yaml
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #19181:
URL: https://github.com/apache/airflow/pull/19181#issuecomment-986186800
Here issue: https://github.com/yannh/kubernetes-json-schema/issues/11
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jedcunningham commented on a change in pull request #19181: More strict schema for values.yaml
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on a change in pull request #19181:
URL: https://github.com/apache/airflow/pull/19181#discussion_r741267980
##########
File path: chart/values.schema.json
##########
@@ -568,6 +601,23 @@
"type": "array",
"x-docsSection": "Airflow",
"default": [],
+ "items": {
+ "type": "object",
+ "properties": {
+ "envName": {
+ "description": "The name of the environment variable under which the secret will be available",
+ "type": "string"
+ },
+ "secretName": {
+ "description": "The name of the Kubernetes secret that will be read",
+ "type": "string"
+ },
+ "secretKey": {
+ "description": "The key of the Kubernetes secret",
+ "type": "string"
+ }
+ }
Review comment:
```suggestion
},
"additionalProperties": False,
"required": ["envName", "secretName"]
```
##########
File path: chart/values.schema.json
##########
@@ -89,13 +89,18 @@
"description": "Specify scheduling constraints for all pods.",
"type": "object",
"default": {},
- "x-docsSection": "Kubernetes"
+ "x-docsSection": "Kubernetes",
+ "$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0/_definitions.json#/definitions/io.k8s.api.core.v1.Affinity"
},
"tolerations": {
"description": "Specify Tolerations for all pods.",
"type": "array",
"default": [],
- "x-docsSection": "Kubernetes"
+ "x-docsSection": "Kubernetes",
+ "items": {
+ "type": "object",
+ "$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0/_definitions.json#/definitions/io.k8s.api.core.v1.Toleration"
Review comment:
Thoughts on using the strict variant? e.g:
https://github.com/yannh/kubernetes-json-schema/blob/master/v1.22.0-standalone-strict/_definitions.json
We are strict by default in our schema, I think it makes sense to continue that trend here too.
##########
File path: chart/values.schema.json
##########
@@ -89,13 +89,18 @@
"description": "Specify scheduling constraints for all pods.",
"type": "object",
"default": {},
- "x-docsSection": "Kubernetes"
+ "x-docsSection": "Kubernetes",
+ "$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0/_definitions.json#/definitions/io.k8s.api.core.v1.Affinity"
},
"tolerations": {
"description": "Specify Tolerations for all pods.",
"type": "array",
"default": [],
- "x-docsSection": "Kubernetes"
+ "x-docsSection": "Kubernetes",
+ "items": {
+ "type": "object",
+ "$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0/_definitions.json#/definitions/io.k8s.api.core.v1.Toleration"
Review comment:
Is there a benefit to using the full `_definitions.json` over using specific schemas? e.g:
https://github.com/yannh/kubernetes-json-schema/blob/master/v1.22.0-standalone-strict/toleration-v1.json
##########
File path: chart/values.schema.json
##########
@@ -2069,11 +2258,38 @@
"annotations": {
"description": "Annotations for the webserver Service.",
"type": "object",
- "default": {}
+ "default": {},
+ "additionalProperties": {
+ "type": "string"
+ }
},
"ports": {
"description": "Ports for the webserver Service.",
"type": "array",
+ "items": {
+ "type": "object",
+ "additionalProperties": false,
+ "properties": {
+ "name": {
+ "type": "string"
+ },
+ "port": {
+ "type": [
+ "string",
+ "integer"
+ ]
+ },
+ "targetPort": {
+ "type": [
+ "string",
+ "integer"
+ ]
+ },
+ "protocol": {
+ "type": "string"
+ }
+ }
+ },
Review comment:
We should be able to use `ServicePort` here (and with flower), no?
```suggestion
"items": {
"$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0/_definitions.json#/definitions/io.k8s.api.core.v1.ServicePort"
},
```
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on pull request #19181: More strict schema for values.yaml
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #19181:
URL: https://github.com/apache/airflow/pull/19181#issuecomment-986107252
I looked at the CI problem and I think the OpenAPI specification has a bug.
ResourceQuotaSpec has the following definition:
https://github.com/yannh/kubernetes-json-schema/blob/master/v1.21.0/resourcequotaspec.json
```json
{
"properties": {
"hard": {
"additionalProperties": {
"$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.21.0/_definitions.json#/definitions/io.k8s.apimachinery.pkg.api.resource.Quantity"
},
"type": [
"object",
"null"
]
},
...
```
and
quantity has been defined as follow:
https://github.com/yannh/kubernetes-json-schema/blob/master/v1.21.0/quantity.json
```json
{
"oneOf": [
{
"type": "string"
},
{
"type": "number"
}
],
"$schema": "http://json-schema.org/schema#",
"type": "object"
}
```
The fields `oneOf` and `type` are mutually exclusive. For now, I've copied the type definitions directly into the spec.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #19181: More strict schema for values.yaml
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #19181:
URL: https://github.com/apache/airflow/pull/19181#discussion_r741610474
##########
File path: chart/values.schema.json
##########
@@ -89,13 +89,18 @@
"description": "Specify scheduling constraints for all pods.",
"type": "object",
"default": {},
- "x-docsSection": "Kubernetes"
+ "x-docsSection": "Kubernetes",
+ "$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0/_definitions.json#/definitions/io.k8s.api.core.v1.Affinity"
},
"tolerations": {
"description": "Specify Tolerations for all pods.",
"type": "array",
"default": [],
- "x-docsSection": "Kubernetes"
+ "x-docsSection": "Kubernetes",
+ "items": {
+ "type": "object",
+ "$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0/_definitions.json#/definitions/io.k8s.api.core.v1.Toleration"
Review comment:
Fie `_definitions.json` is easier to browse as we can use CTRL + F to search by field name.
##########
File path: chart/values.schema.json
##########
@@ -2069,11 +2258,38 @@
"annotations": {
"description": "Annotations for the webserver Service.",
"type": "object",
- "default": {}
+ "default": {},
+ "additionalProperties": {
+ "type": "string"
+ }
},
"ports": {
"description": "Ports for the webserver Service.",
"type": "array",
+ "items": {
+ "type": "object",
+ "additionalProperties": false,
+ "properties": {
+ "name": {
+ "type": "string"
+ },
+ "port": {
+ "type": [
+ "string",
+ "integer"
+ ]
+ },
+ "targetPort": {
+ "type": [
+ "string",
+ "integer"
+ ]
+ },
+ "protocol": {
+ "type": "string"
+ }
+ }
+ },
Review comment:
No. It is not compatible with current default value:
```
{
"name": "airflow-ui",
"port": "{{ .Values.ports.airflowUI }}"
}
```
In the Kubernetes schema, the `port` field is defined as `integer`, but we use `string`.
##########
File path: chart/values.schema.json
##########
@@ -568,6 +601,23 @@
"type": "array",
"x-docsSection": "Airflow",
"default": [],
+ "items": {
+ "type": "object",
+ "properties": {
+ "envName": {
+ "description": "The name of the environment variable under which the secret will be available",
+ "type": "string"
+ },
+ "secretName": {
+ "description": "The name of the Kubernetes secret that will be read",
+ "type": "string"
+ },
+ "secretKey": {
+ "description": "The key of the Kubernetes secret",
+ "type": "string"
+ }
+ }
Review comment:
Thanks. I also checked and ` "additionalProperties": false,` was missing from the other definitions as well.
##########
File path: chart/values.schema.json
##########
@@ -89,13 +89,18 @@
"description": "Specify scheduling constraints for all pods.",
"type": "object",
"default": {},
- "x-docsSection": "Kubernetes"
+ "x-docsSection": "Kubernetes",
+ "$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0/_definitions.json#/definitions/io.k8s.api.core.v1.Affinity"
},
"tolerations": {
"description": "Specify Tolerations for all pods.",
"type": "array",
"default": [],
- "x-docsSection": "Kubernetes"
+ "x-docsSection": "Kubernetes",
+ "items": {
+ "type": "object",
+ "$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0/_definitions.json#/definitions/io.k8s.api.core.v1.Toleration"
Review comment:
Good idea. Updated. I added `"additionalProperties": false,` in a few place also.
##########
File path: chart/values.schema.json
##########
@@ -89,13 +89,18 @@
"description": "Specify scheduling constraints for all pods.",
"type": "object",
"default": {},
- "x-docsSection": "Kubernetes"
+ "x-docsSection": "Kubernetes",
+ "$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0/_definitions.json#/definitions/io.k8s.api.core.v1.Affinity"
},
"tolerations": {
"description": "Specify Tolerations for all pods.",
"type": "array",
"default": [],
- "x-docsSection": "Kubernetes"
+ "x-docsSection": "Kubernetes",
+ "items": {
+ "type": "object",
+ "$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0/_definitions.json#/definitions/io.k8s.api.core.v1.Toleration"
Review comment:
I checked, and we use a very narrow subset of object types, so it's better to fetch each definition separately.
```
cat chart/values.schema.json | jq '.. | objects| .["$ref"] | strings' | sort | uniq
"https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0-standalone-strict/_definitions.json#/definitions/io.k8s.api.apps.v1.DeploymentStrategy"
"https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0-standalone-strict/_definitions.json#/definitions/io.k8s.api.core.v1.Affinity"
"https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0-standalone-strict/_definitions.json#/definitions/io.k8s.api.core.v1.Container"
"https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0-standalone-strict/_definitions.json#/definitions/io.k8s.api.core.v1.HostAlias"
"https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0-standalone-strict/_definitions.json#/definitions/io.k8s.api.core.v1.LimitRangeItem"
"https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0-standalone-strict/_definitions.json#/definitions/io.k8s.api.core.v1.ResourceRequirements"
"https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0-standalone-strict/_definitions.json#/definitions/io.k8s.api.core.v1.Toleration"
"https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0-standalone-strict/_definitions.json#/definitions/io.k8s.api.core.v1.Volume"
"https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0-standalone-strict/_definitions.json#/definitions/io.k8s.api.core.v1.VolumeMount"
"https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0-standalone-strict/_definitions.json#/definitions/io.k8s.api.networking.v1.NetworkPolicyPeer"
"https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0-standalone-strict/_definitions.json#/definitions/io.k8s.api.networking.v1.NetworkPolicyPort"
"https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0-standalone-strict/_definitions.json#/definitions/io.k8s.apimachinery.pkg.api.resource.Quantity"
```
##########
File path: chart/tests/test_flower.py
##########
@@ -358,18 +358,18 @@ def test_defaults(self):
"ports, expected_ports",
[
(
- [{"name": "{{ .Release.Name }}", "protocol": "UDP", "port": "{{ .Values.ports.flowerUI }}"}],
- [{"name": "RELEASE-NAME", "protocol": "UDP", "port": 5555}],
+ [{"protocol": "UDP", "port": "{{ .Values.ports.flowerUI }}"}],
+ [{"protocol": "UDP", "port": 5555}],
),
- ([{"name": "only_sidecar", "port": "sidecar"}], [{"name": "only_sidecar", "port": "sidecar"}]),
+ ([{"port": "sidecar"}], [{"port": "sidecar"}]),
(
[
- {"name": "flower-ui", "port": "{{ .Values.ports.flowerUI }}"},
- {"name": "sidecar", "port": 80},
+ {"port": "{{ .Values.ports.flowerUI }}"},
+ {"port": 80},
Review comment:
This is a test for NetworkPolicy, not Service. In NetworkPolicy, we use [https://github.com/yannh/kubernetes-json-schema/blob/master/v1.21.0/networkpolicyport-networking-v1.json](NetworkPolicyPort), not [ServicePort](https://github.com/yannh/kubernetes-json-schema/blob/master/v1.21.0/serviceport-v1.json), so `name` field is illegal here.
##########
File path: chart/tests/test_flower.py
##########
@@ -358,18 +358,18 @@ def test_defaults(self):
"ports, expected_ports",
[
(
- [{"name": "{{ .Release.Name }}", "protocol": "UDP", "port": "{{ .Values.ports.flowerUI }}"}],
- [{"name": "RELEASE-NAME", "protocol": "UDP", "port": 5555}],
+ [{"protocol": "UDP", "port": "{{ .Values.ports.flowerUI }}"}],
+ [{"protocol": "UDP", "port": 5555}],
),
- ([{"name": "only_sidecar", "port": "sidecar"}], [{"name": "only_sidecar", "port": "sidecar"}]),
+ ([{"port": "sidecar"}], [{"port": "sidecar"}]),
(
[
- {"name": "flower-ui", "port": "{{ .Values.ports.flowerUI }}"},
- {"name": "sidecar", "port": 80},
+ {"port": "{{ .Values.ports.flowerUI }}"},
+ {"port": 80},
Review comment:
This is a test for NetworkPolicy, not Service. In NetworkPolicy, we use [NetworkPolicyPort](https://github.com/yannh/kubernetes-json-schema/blob/master/v1.21.0/serviceport-v1.json), not [ServicePort](https://github.com/yannh/kubernetes-json-schema/blob/master/v1.21.0/serviceport-v1.json), so `name` field is illegal here.
##########
File path: chart/tests/test_flower.py
##########
@@ -358,18 +358,18 @@ def test_defaults(self):
"ports, expected_ports",
[
(
- [{"name": "{{ .Release.Name }}", "protocol": "UDP", "port": "{{ .Values.ports.flowerUI }}"}],
- [{"name": "RELEASE-NAME", "protocol": "UDP", "port": 5555}],
+ [{"protocol": "UDP", "port": "{{ .Values.ports.flowerUI }}"}],
+ [{"protocol": "UDP", "port": 5555}],
),
- ([{"name": "only_sidecar", "port": "sidecar"}], [{"name": "only_sidecar", "port": "sidecar"}]),
+ ([{"port": "sidecar"}], [{"port": "sidecar"}]),
(
[
- {"name": "flower-ui", "port": "{{ .Values.ports.flowerUI }}"},
- {"name": "sidecar", "port": 80},
+ {"port": "{{ .Values.ports.flowerUI }}"},
+ {"port": 80},
Review comment:
This is a test for NetworkPolicy, not Service. In NetworkPolicy, we use [NetworkPolicyPort](https://github.com/yannh/kubernetes-json-schema/blob/master/v1.21.0/networkpolicyport-networking-v1.json), not [ServicePort](https://github.com/yannh/kubernetes-json-schema/blob/master/v1.21.0/serviceport-v1.json), so `name` field is illegal 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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #19181: More strict schema for values.yaml
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #19181:
URL: https://github.com/apache/airflow/pull/19181#issuecomment-959419084
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #19181: More strict schema for values.yaml
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #19181:
URL: https://github.com/apache/airflow/pull/19181#issuecomment-959419084
The PR is likely OK to be merged with just subset of tests for default Python and Database versions without running the full matrix of tests, because it does not modify the core of Airflow. If the committers decide that the full tests matrix is needed, they will add the label 'full tests needed'. Then you should rebase to the latest main or amend the last commit of the PR, and push it with --force-with-lease.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj edited a comment on pull request #19181: More strict schema for values.yaml
Posted by GitBox <gi...@apache.org>.
mik-laj edited a comment on pull request #19181:
URL: https://github.com/apache/airflow/pull/19181#issuecomment-950148723
We are now only missing a few types to have full and strict coverage.
```
$ cat chart/values.schema.json | jq 'recurse | objects | select(.type == "array") | select(.items == null)' | jq -s 'length'
10
$ cat chart/values.schema.json | jq 'recurse | objects | select(.type == "object") | select(.properties == null) | select(.["$ref"] == null) | select(.additionalProperties == null)' | jq -s 'length'
3
```
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on pull request #19181: More strict schema for values.yaml
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #19181:
URL: https://github.com/apache/airflow/pull/19181#issuecomment-950148723
We do not lack many types now to be fully covered with the structure.
```
$ cat chart/values.schema.json | jq 'recurse | objects | select(.type == "array") | select(.items == null)' | jq -s 'length'
10
$ cat chart/values.schema.json | jq 'recurse | objects | select(.type == "object") | select(.properties == null) | select(.["$ref"] == null) | select(.additionalProperties == null)' | jq -s 'length'
3
```
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jedcunningham commented on a change in pull request #19181: More strict schema for values.yaml
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on a change in pull request #19181:
URL: https://github.com/apache/airflow/pull/19181#discussion_r741267980
##########
File path: chart/values.schema.json
##########
@@ -568,6 +601,23 @@
"type": "array",
"x-docsSection": "Airflow",
"default": [],
+ "items": {
+ "type": "object",
+ "properties": {
+ "envName": {
+ "description": "The name of the environment variable under which the secret will be available",
+ "type": "string"
+ },
+ "secretName": {
+ "description": "The name of the Kubernetes secret that will be read",
+ "type": "string"
+ },
+ "secretKey": {
+ "description": "The key of the Kubernetes secret",
+ "type": "string"
+ }
+ }
Review comment:
```suggestion
},
"additionalProperties": False,
"required": ["envName", "secretName"]
```
##########
File path: chart/values.schema.json
##########
@@ -89,13 +89,18 @@
"description": "Specify scheduling constraints for all pods.",
"type": "object",
"default": {},
- "x-docsSection": "Kubernetes"
+ "x-docsSection": "Kubernetes",
+ "$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0/_definitions.json#/definitions/io.k8s.api.core.v1.Affinity"
},
"tolerations": {
"description": "Specify Tolerations for all pods.",
"type": "array",
"default": [],
- "x-docsSection": "Kubernetes"
+ "x-docsSection": "Kubernetes",
+ "items": {
+ "type": "object",
+ "$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0/_definitions.json#/definitions/io.k8s.api.core.v1.Toleration"
Review comment:
Thoughts on using the strict variant? e.g:
https://github.com/yannh/kubernetes-json-schema/blob/master/v1.22.0-standalone-strict/_definitions.json
We are strict by default in our schema, I think it makes sense to continue that trend here too.
##########
File path: chart/values.schema.json
##########
@@ -89,13 +89,18 @@
"description": "Specify scheduling constraints for all pods.",
"type": "object",
"default": {},
- "x-docsSection": "Kubernetes"
+ "x-docsSection": "Kubernetes",
+ "$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0/_definitions.json#/definitions/io.k8s.api.core.v1.Affinity"
},
"tolerations": {
"description": "Specify Tolerations for all pods.",
"type": "array",
"default": [],
- "x-docsSection": "Kubernetes"
+ "x-docsSection": "Kubernetes",
+ "items": {
+ "type": "object",
+ "$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0/_definitions.json#/definitions/io.k8s.api.core.v1.Toleration"
Review comment:
Is there a benefit to using the full `_definitions.json` over using specific schemas? e.g:
https://github.com/yannh/kubernetes-json-schema/blob/master/v1.22.0-standalone-strict/toleration-v1.json
##########
File path: chart/values.schema.json
##########
@@ -2069,11 +2258,38 @@
"annotations": {
"description": "Annotations for the webserver Service.",
"type": "object",
- "default": {}
+ "default": {},
+ "additionalProperties": {
+ "type": "string"
+ }
},
"ports": {
"description": "Ports for the webserver Service.",
"type": "array",
+ "items": {
+ "type": "object",
+ "additionalProperties": false,
+ "properties": {
+ "name": {
+ "type": "string"
+ },
+ "port": {
+ "type": [
+ "string",
+ "integer"
+ ]
+ },
+ "targetPort": {
+ "type": [
+ "string",
+ "integer"
+ ]
+ },
+ "protocol": {
+ "type": "string"
+ }
+ }
+ },
Review comment:
We should be able to use `ServicePort` here (and with flower), no?
```suggestion
"items": {
"$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0/_definitions.json#/definitions/io.k8s.api.core.v1.ServicePort"
},
```
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on pull request #19181: More strict schema for values.yaml
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #19181:
URL: https://github.com/apache/airflow/pull/19181#issuecomment-957070404
I have resolved all the problems except the following
```
$ cat chart/values.schema.json | jq 'recurse | objects | select(.type == "array") | select(.items == null)'
{
"description": "HTTP paths to add to the web Ingress before the default path.",
"type": "array",
"default": []
}
{
"description": "HTTP paths to add to the web Ingress after the default path.",
"type": "array",
"default": []
}
{
"description": "Additional mappings for StatsD exporter.",
"type": "array",
"default": []
}
```
1 and 2: I don't know the correct structure. This field is also not covered by tests.
3: I did not find the schema.json file for this parameter.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on pull request #19181: More strict schema for values.yaml
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #19181:
URL: https://github.com/apache/airflow/pull/19181#issuecomment-957070404
I have resolved all the problems except the following
```
$ cat chart/values.schema.json | jq 'recurse | objects | select(.type == "array") | select(.items == null)'
{
"description": "HTTP paths to add to the web Ingress before the default path.",
"type": "array",
"default": []
}
{
"description": "HTTP paths to add to the web Ingress after the default path.",
"type": "array",
"default": []
}
{
"description": "Additional mappings for StatsD exporter.",
"type": "array",
"default": []
}
```
1 and 2: I don't know the correct structure. This field is also not covered by tests.
3: I did not find the schema.json file for this parameter.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on pull request #19181: More strict schema for values.yaml
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #19181:
URL: https://github.com/apache/airflow/pull/19181#issuecomment-959732808
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on pull request #19181: More strict schema for values.yaml
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #19181:
URL: https://github.com/apache/airflow/pull/19181#issuecomment-959732808
> I could easily see folks having extra config in their values and this stricter schema being surprising. Thoughts?
As for fields that have a structure defined with an external reference, users cannot define additional fields as this will cause an invalid k8s object to be generated. Here is an example: https://github.com/apache/airflow/pull/19379
The problem is only with the objects that are defined in this schema, but we can remove them from this contribution and change them in the future.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #19181: More strict schema for values.yaml
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #19181:
URL: https://github.com/apache/airflow/pull/19181#discussion_r741615115
##########
File path: chart/values.schema.json
##########
@@ -568,6 +601,23 @@
"type": "array",
"x-docsSection": "Airflow",
"default": [],
+ "items": {
+ "type": "object",
+ "properties": {
+ "envName": {
+ "description": "The name of the environment variable under which the secret will be available",
+ "type": "string"
+ },
+ "secretName": {
+ "description": "The name of the Kubernetes secret that will be read",
+ "type": "string"
+ },
+ "secretKey": {
+ "description": "The key of the Kubernetes secret",
+ "type": "string"
+ }
+ }
Review comment:
Thanks. I also checked and ` "additionalProperties": false,` was missing from the other definitions as well.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #19181: More strict schema for values.yaml
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #19181:
URL: https://github.com/apache/airflow/pull/19181#discussion_r742133261
##########
File path: chart/tests/test_flower.py
##########
@@ -358,18 +358,18 @@ def test_defaults(self):
"ports, expected_ports",
[
(
- [{"name": "{{ .Release.Name }}", "protocol": "UDP", "port": "{{ .Values.ports.flowerUI }}"}],
- [{"name": "RELEASE-NAME", "protocol": "UDP", "port": 5555}],
+ [{"protocol": "UDP", "port": "{{ .Values.ports.flowerUI }}"}],
+ [{"protocol": "UDP", "port": 5555}],
),
- ([{"name": "only_sidecar", "port": "sidecar"}], [{"name": "only_sidecar", "port": "sidecar"}]),
+ ([{"port": "sidecar"}], [{"port": "sidecar"}]),
(
[
- {"name": "flower-ui", "port": "{{ .Values.ports.flowerUI }}"},
- {"name": "sidecar", "port": 80},
+ {"port": "{{ .Values.ports.flowerUI }}"},
+ {"port": 80},
Review comment:
This is a test for NetworkPolicy, not Service. In NetworkPolicy, we use [NetworkPolicyPort](https://github.com/yannh/kubernetes-json-schema/blob/master/v1.21.0/networkpolicyport-networking-v1.json), not [ServicePort](https://github.com/yannh/kubernetes-json-schema/blob/master/v1.21.0/serviceport-v1.json), so `name` field is illegal 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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #19181: More strict schema for values.yaml
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #19181:
URL: https://github.com/apache/airflow/pull/19181#discussion_r741626451
##########
File path: chart/values.schema.json
##########
@@ -89,13 +89,18 @@
"description": "Specify scheduling constraints for all pods.",
"type": "object",
"default": {},
- "x-docsSection": "Kubernetes"
+ "x-docsSection": "Kubernetes",
+ "$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0/_definitions.json#/definitions/io.k8s.api.core.v1.Affinity"
},
"tolerations": {
"description": "Specify Tolerations for all pods.",
"type": "array",
"default": [],
- "x-docsSection": "Kubernetes"
+ "x-docsSection": "Kubernetes",
+ "items": {
+ "type": "object",
+ "$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0/_definitions.json#/definitions/io.k8s.api.core.v1.Toleration"
Review comment:
I checked, and we use a very narrow subset of object types, so it's better to fetch each definition separately.
```
cat chart/values.schema.json | jq '.. | objects| .["$ref"] | strings' | sort | uniq
"https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0-standalone-strict/_definitions.json#/definitions/io.k8s.api.apps.v1.DeploymentStrategy"
"https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0-standalone-strict/_definitions.json#/definitions/io.k8s.api.core.v1.Affinity"
"https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0-standalone-strict/_definitions.json#/definitions/io.k8s.api.core.v1.Container"
"https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0-standalone-strict/_definitions.json#/definitions/io.k8s.api.core.v1.HostAlias"
"https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0-standalone-strict/_definitions.json#/definitions/io.k8s.api.core.v1.LimitRangeItem"
"https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0-standalone-strict/_definitions.json#/definitions/io.k8s.api.core.v1.ResourceRequirements"
"https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0-standalone-strict/_definitions.json#/definitions/io.k8s.api.core.v1.Toleration"
"https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0-standalone-strict/_definitions.json#/definitions/io.k8s.api.core.v1.Volume"
"https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0-standalone-strict/_definitions.json#/definitions/io.k8s.api.core.v1.VolumeMount"
"https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0-standalone-strict/_definitions.json#/definitions/io.k8s.api.networking.v1.NetworkPolicyPeer"
"https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0-standalone-strict/_definitions.json#/definitions/io.k8s.api.networking.v1.NetworkPolicyPort"
"https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0-standalone-strict/_definitions.json#/definitions/io.k8s.apimachinery.pkg.api.resource.Quantity"
```
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj edited a comment on pull request #19181: More strict schema for values.yaml
Posted by GitBox <gi...@apache.org>.
mik-laj edited a comment on pull request #19181:
URL: https://github.com/apache/airflow/pull/19181#issuecomment-950148723
We are now only missing a few types to have full and strict coverage.
Before:
```
$ cat chart/values.schema.json | jq 'recurse | objects | select(.type == "object") | select(.properties == null) | select(.["$ref"] == null) | select(.additionalProperties == null)' | jq -s 'length'
57
$ cat chart/values.schema.json | jq 'recurse | objects | select(.type == "array") | select(.items == null)' | jq -s 'length'
51
```
After:
```
$ cat chart/values.schema.json | jq 'recurse | objects | select(.type == "object") | select(.properties == null) | select(.["$ref"] == null) | select(.additionalProperties == null)' | jq -s 'length'
3
$ cat chart/values.schema.json | jq 'recurse | objects | select(.type == "array") | select(.items == null)' | jq -s 'length'
10
```
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj edited a comment on pull request #19181: More strict schema for values.yaml
Posted by GitBox <gi...@apache.org>.
mik-laj edited a comment on pull request #19181:
URL: https://github.com/apache/airflow/pull/19181#issuecomment-950148723
We are now only missing a few types to have full and strict coverage.
Before:
```
$ cat chart/values.schema.json | jq 'recurse | objects | select(.type == "object") | select(.properties == null) | select(.["$ref"] == null) | select(.additionalProperties == null)' | jq -s 'length'
57
$ cat chart/values.schema.json | jq 'recurse | objects | select(.type == "array") | select(.items == null)' | jq -s 'length'
51
```
After:
```
$ cat chart/values.schema.json | jq 'recurse | objects | select(.type == "object") | select(.properties == null) | select(.["$ref"] == null) | select(.additionalProperties == null)' | jq -s 'length'
3
$ cat chart/values.schema.json | jq 'recurse | objects | select(.type == "array") | select(.items == null)' | jq -s 'length'
10
```
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj edited a comment on pull request #19181: More strict schema for values.yaml
Posted by GitBox <gi...@apache.org>.
mik-laj edited a comment on pull request #19181:
URL: https://github.com/apache/airflow/pull/19181#issuecomment-950148723
We are now only missing a few types to have full and strict coverage.
Before:
```
$ cat chart/values.schema.json | jq 'recurse | objects | select(.type == "object") | select(.properties == null) | select(.["$ref"] == null) | select(.additionalProperties == null)' | jq -s 'length'
57
$ cat chart/values.schema.json | jq 'recurse | objects | select(.type == "array") | select(.items == null)' | jq -s 'length'
51
```
After:
```
$ cat chart/values.schema.json | jq 'recurse | objects | select(.type == "object") | select(.properties == null) | select(.["$ref"] == null) | select(.additionalProperties == null)' | jq -s 'length'
3
$ cat chart/values.schema.json | jq 'recurse | objects | select(.type == "array") | select(.items == null)' | jq -s 'length'
10
```
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on pull request #19181: More strict schema for values.yaml
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #19181:
URL: https://github.com/apache/airflow/pull/19181#issuecomment-950975645
@mrbaguvix I have currently solved all the issues with the structure of objects. Now only the issues with the structures of the array remain, but I think we can solve that as a separate contribution. It is important to me that most of the problems are resolved, so following contributions that follow the conventions in this file will also declare the complete structure of the objects. Previously, even when someone wanted to use a reference, they weren't sure if it was a good idea. Here is example: https://github.com/apache/airflow/pull/19175#discussion_r734891083
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj merged pull request #19181: Strict schema for k8s objects for values.yaml
Posted by GitBox <gi...@apache.org>.
mik-laj merged pull request #19181:
URL: https://github.com/apache/airflow/pull/19181
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #19181: More strict schema for values.yaml
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #19181:
URL: https://github.com/apache/airflow/pull/19181#discussion_r741610474
##########
File path: chart/values.schema.json
##########
@@ -89,13 +89,18 @@
"description": "Specify scheduling constraints for all pods.",
"type": "object",
"default": {},
- "x-docsSection": "Kubernetes"
+ "x-docsSection": "Kubernetes",
+ "$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0/_definitions.json#/definitions/io.k8s.api.core.v1.Affinity"
},
"tolerations": {
"description": "Specify Tolerations for all pods.",
"type": "array",
"default": [],
- "x-docsSection": "Kubernetes"
+ "x-docsSection": "Kubernetes",
+ "items": {
+ "type": "object",
+ "$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0/_definitions.json#/definitions/io.k8s.api.core.v1.Toleration"
Review comment:
Fie `_definitions.json` is easier to browse as we can use CTRL + F to search by field name.
##########
File path: chart/values.schema.json
##########
@@ -2069,11 +2258,38 @@
"annotations": {
"description": "Annotations for the webserver Service.",
"type": "object",
- "default": {}
+ "default": {},
+ "additionalProperties": {
+ "type": "string"
+ }
},
"ports": {
"description": "Ports for the webserver Service.",
"type": "array",
+ "items": {
+ "type": "object",
+ "additionalProperties": false,
+ "properties": {
+ "name": {
+ "type": "string"
+ },
+ "port": {
+ "type": [
+ "string",
+ "integer"
+ ]
+ },
+ "targetPort": {
+ "type": [
+ "string",
+ "integer"
+ ]
+ },
+ "protocol": {
+ "type": "string"
+ }
+ }
+ },
Review comment:
No. It is not compatible with current default value:
```
{
"name": "airflow-ui",
"port": "{{ .Values.ports.airflowUI }}"
}
```
In the Kubernetes schema, the `port` field is defined as `integer`, but we use `string`.
##########
File path: chart/values.schema.json
##########
@@ -568,6 +601,23 @@
"type": "array",
"x-docsSection": "Airflow",
"default": [],
+ "items": {
+ "type": "object",
+ "properties": {
+ "envName": {
+ "description": "The name of the environment variable under which the secret will be available",
+ "type": "string"
+ },
+ "secretName": {
+ "description": "The name of the Kubernetes secret that will be read",
+ "type": "string"
+ },
+ "secretKey": {
+ "description": "The key of the Kubernetes secret",
+ "type": "string"
+ }
+ }
Review comment:
Thanks. I also checked and ` "additionalProperties": false,` was missing from the other definitions as well.
##########
File path: chart/values.schema.json
##########
@@ -89,13 +89,18 @@
"description": "Specify scheduling constraints for all pods.",
"type": "object",
"default": {},
- "x-docsSection": "Kubernetes"
+ "x-docsSection": "Kubernetes",
+ "$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0/_definitions.json#/definitions/io.k8s.api.core.v1.Affinity"
},
"tolerations": {
"description": "Specify Tolerations for all pods.",
"type": "array",
"default": [],
- "x-docsSection": "Kubernetes"
+ "x-docsSection": "Kubernetes",
+ "items": {
+ "type": "object",
+ "$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0/_definitions.json#/definitions/io.k8s.api.core.v1.Toleration"
Review comment:
Good idea. Updated. I added `"additionalProperties": false,` in a few place also.
##########
File path: chart/values.schema.json
##########
@@ -89,13 +89,18 @@
"description": "Specify scheduling constraints for all pods.",
"type": "object",
"default": {},
- "x-docsSection": "Kubernetes"
+ "x-docsSection": "Kubernetes",
+ "$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0/_definitions.json#/definitions/io.k8s.api.core.v1.Affinity"
},
"tolerations": {
"description": "Specify Tolerations for all pods.",
"type": "array",
"default": [],
- "x-docsSection": "Kubernetes"
+ "x-docsSection": "Kubernetes",
+ "items": {
+ "type": "object",
+ "$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0/_definitions.json#/definitions/io.k8s.api.core.v1.Toleration"
Review comment:
I checked, and we use a very narrow subset of object types, so it's better to fetch each definition separately.
```
cat chart/values.schema.json | jq '.. | objects| .["$ref"] | strings' | sort | uniq
"https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0-standalone-strict/_definitions.json#/definitions/io.k8s.api.apps.v1.DeploymentStrategy"
"https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0-standalone-strict/_definitions.json#/definitions/io.k8s.api.core.v1.Affinity"
"https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0-standalone-strict/_definitions.json#/definitions/io.k8s.api.core.v1.Container"
"https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0-standalone-strict/_definitions.json#/definitions/io.k8s.api.core.v1.HostAlias"
"https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0-standalone-strict/_definitions.json#/definitions/io.k8s.api.core.v1.LimitRangeItem"
"https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0-standalone-strict/_definitions.json#/definitions/io.k8s.api.core.v1.ResourceRequirements"
"https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0-standalone-strict/_definitions.json#/definitions/io.k8s.api.core.v1.Toleration"
"https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0-standalone-strict/_definitions.json#/definitions/io.k8s.api.core.v1.Volume"
"https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0-standalone-strict/_definitions.json#/definitions/io.k8s.api.core.v1.VolumeMount"
"https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0-standalone-strict/_definitions.json#/definitions/io.k8s.api.networking.v1.NetworkPolicyPeer"
"https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0-standalone-strict/_definitions.json#/definitions/io.k8s.api.networking.v1.NetworkPolicyPort"
"https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0-standalone-strict/_definitions.json#/definitions/io.k8s.apimachinery.pkg.api.resource.Quantity"
```
##########
File path: chart/tests/test_flower.py
##########
@@ -358,18 +358,18 @@ def test_defaults(self):
"ports, expected_ports",
[
(
- [{"name": "{{ .Release.Name }}", "protocol": "UDP", "port": "{{ .Values.ports.flowerUI }}"}],
- [{"name": "RELEASE-NAME", "protocol": "UDP", "port": 5555}],
+ [{"protocol": "UDP", "port": "{{ .Values.ports.flowerUI }}"}],
+ [{"protocol": "UDP", "port": 5555}],
),
- ([{"name": "only_sidecar", "port": "sidecar"}], [{"name": "only_sidecar", "port": "sidecar"}]),
+ ([{"port": "sidecar"}], [{"port": "sidecar"}]),
(
[
- {"name": "flower-ui", "port": "{{ .Values.ports.flowerUI }}"},
- {"name": "sidecar", "port": 80},
+ {"port": "{{ .Values.ports.flowerUI }}"},
+ {"port": 80},
Review comment:
This is a test for NetworkPolicy, not Service. In NetworkPolicy, we use [https://github.com/yannh/kubernetes-json-schema/blob/master/v1.21.0/networkpolicyport-networking-v1.json](NetworkPolicyPort), not [ServicePort](https://github.com/yannh/kubernetes-json-schema/blob/master/v1.21.0/serviceport-v1.json), so `name` field is illegal here.
##########
File path: chart/tests/test_flower.py
##########
@@ -358,18 +358,18 @@ def test_defaults(self):
"ports, expected_ports",
[
(
- [{"name": "{{ .Release.Name }}", "protocol": "UDP", "port": "{{ .Values.ports.flowerUI }}"}],
- [{"name": "RELEASE-NAME", "protocol": "UDP", "port": 5555}],
+ [{"protocol": "UDP", "port": "{{ .Values.ports.flowerUI }}"}],
+ [{"protocol": "UDP", "port": 5555}],
),
- ([{"name": "only_sidecar", "port": "sidecar"}], [{"name": "only_sidecar", "port": "sidecar"}]),
+ ([{"port": "sidecar"}], [{"port": "sidecar"}]),
(
[
- {"name": "flower-ui", "port": "{{ .Values.ports.flowerUI }}"},
- {"name": "sidecar", "port": 80},
+ {"port": "{{ .Values.ports.flowerUI }}"},
+ {"port": 80},
Review comment:
This is a test for NetworkPolicy, not Service. In NetworkPolicy, we use [NetworkPolicyPort](https://github.com/yannh/kubernetes-json-schema/blob/master/v1.21.0/serviceport-v1.json), not [ServicePort](https://github.com/yannh/kubernetes-json-schema/blob/master/v1.21.0/serviceport-v1.json), so `name` field is illegal here.
##########
File path: chart/tests/test_flower.py
##########
@@ -358,18 +358,18 @@ def test_defaults(self):
"ports, expected_ports",
[
(
- [{"name": "{{ .Release.Name }}", "protocol": "UDP", "port": "{{ .Values.ports.flowerUI }}"}],
- [{"name": "RELEASE-NAME", "protocol": "UDP", "port": 5555}],
+ [{"protocol": "UDP", "port": "{{ .Values.ports.flowerUI }}"}],
+ [{"protocol": "UDP", "port": 5555}],
),
- ([{"name": "only_sidecar", "port": "sidecar"}], [{"name": "only_sidecar", "port": "sidecar"}]),
+ ([{"port": "sidecar"}], [{"port": "sidecar"}]),
(
[
- {"name": "flower-ui", "port": "{{ .Values.ports.flowerUI }}"},
- {"name": "sidecar", "port": 80},
+ {"port": "{{ .Values.ports.flowerUI }}"},
+ {"port": 80},
Review comment:
This is a test for NetworkPolicy, not Service. In NetworkPolicy, we use [NetworkPolicyPort](https://github.com/yannh/kubernetes-json-schema/blob/master/v1.21.0/networkpolicyport-networking-v1.json), not [ServicePort](https://github.com/yannh/kubernetes-json-schema/blob/master/v1.21.0/serviceport-v1.json), so `name` field is illegal 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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #19181: More strict schema for values.yaml
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #19181:
URL: https://github.com/apache/airflow/pull/19181#discussion_r741610474
##########
File path: chart/values.schema.json
##########
@@ -89,13 +89,18 @@
"description": "Specify scheduling constraints for all pods.",
"type": "object",
"default": {},
- "x-docsSection": "Kubernetes"
+ "x-docsSection": "Kubernetes",
+ "$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0/_definitions.json#/definitions/io.k8s.api.core.v1.Affinity"
},
"tolerations": {
"description": "Specify Tolerations for all pods.",
"type": "array",
"default": [],
- "x-docsSection": "Kubernetes"
+ "x-docsSection": "Kubernetes",
+ "items": {
+ "type": "object",
+ "$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0/_definitions.json#/definitions/io.k8s.api.core.v1.Toleration"
Review comment:
Fie `_definitions.json` is easier to browse as we can use CTRL + F to search by field name.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #19181: More strict schema for values.yaml
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #19181:
URL: https://github.com/apache/airflow/pull/19181#discussion_r741616457
##########
File path: chart/values.schema.json
##########
@@ -89,13 +89,18 @@
"description": "Specify scheduling constraints for all pods.",
"type": "object",
"default": {},
- "x-docsSection": "Kubernetes"
+ "x-docsSection": "Kubernetes",
+ "$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0/_definitions.json#/definitions/io.k8s.api.core.v1.Affinity"
},
"tolerations": {
"description": "Specify Tolerations for all pods.",
"type": "array",
"default": [],
- "x-docsSection": "Kubernetes"
+ "x-docsSection": "Kubernetes",
+ "items": {
+ "type": "object",
+ "$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0/_definitions.json#/definitions/io.k8s.api.core.v1.Toleration"
Review comment:
Good idea. Updated. I added `"additionalProperties": false,` in a few place also.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on pull request #19181: More strict schema for values.yaml
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #19181:
URL: https://github.com/apache/airflow/pull/19181#issuecomment-950149788
CC: @simplylizz
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj edited a comment on pull request #19181: More strict schema for values.yaml
Posted by GitBox <gi...@apache.org>.
mik-laj edited a comment on pull request #19181:
URL: https://github.com/apache/airflow/pull/19181#issuecomment-950148723
We are now only missing a few types to have full and strict coverage.
Before:
```
$ cat chart/values.schema.json | jq 'recurse | objects | select(.type == "object") | select(.properties == null) | select(.["$ref"] == null) | select(.additionalProperties == null)' | jq -s 'length'
57
$ cat chart/values.schema.json | jq 'recurse | objects | select(.type == "array") | select(.items == null)' | jq -s 'length'
51
```
After:
```
$ cat chart/values.schema.json | jq 'recurse | objects | select(.type == "array") | select(.items == null)' | jq -s 'length'
10
$ cat chart/values.schema.json | jq 'recurse | objects | select(.type == "object") | select(.properties == null) | select(.["$ref"] == null) | select(.additionalProperties == null)' | jq -s 'length'
3
```
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on pull request #19181: More strict schema for values.yaml
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #19181:
URL: https://github.com/apache/airflow/pull/19181#issuecomment-959732808
> I could easily see folks having extra config in their values and this stricter schema being surprising. Thoughts?
As for fields that have a structure defined with an external reference, users cannot define additional fields as this will cause an invalid k8s object to be generated. Here is an example: https://github.com/apache/airflow/pull/19379
The problem is only with the objects that are defined in this schema, but we can remove them from this contribution and change them in the future.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jedcunningham commented on a change in pull request #19181: More strict schema for values.yaml
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on a change in pull request #19181:
URL: https://github.com/apache/airflow/pull/19181#discussion_r742021449
##########
File path: chart/values.schema.json
##########
@@ -2069,11 +2258,38 @@
"annotations": {
"description": "Annotations for the webserver Service.",
"type": "object",
- "default": {}
+ "default": {},
+ "additionalProperties": {
+ "type": "string"
+ }
},
"ports": {
"description": "Ports for the webserver Service.",
"type": "array",
+ "items": {
+ "type": "object",
+ "additionalProperties": false,
+ "properties": {
+ "name": {
+ "type": "string"
+ },
+ "port": {
+ "type": [
+ "string",
+ "integer"
+ ]
+ },
+ "targetPort": {
+ "type": [
+ "string",
+ "integer"
+ ]
+ },
+ "protocol": {
+ "type": "string"
+ }
+ }
+ },
Review comment:
Ah, good catch.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jedcunningham commented on a change in pull request #19181: More strict schema for values.yaml
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on a change in pull request #19181:
URL: https://github.com/apache/airflow/pull/19181#discussion_r741267980
##########
File path: chart/values.schema.json
##########
@@ -568,6 +601,23 @@
"type": "array",
"x-docsSection": "Airflow",
"default": [],
+ "items": {
+ "type": "object",
+ "properties": {
+ "envName": {
+ "description": "The name of the environment variable under which the secret will be available",
+ "type": "string"
+ },
+ "secretName": {
+ "description": "The name of the Kubernetes secret that will be read",
+ "type": "string"
+ },
+ "secretKey": {
+ "description": "The key of the Kubernetes secret",
+ "type": "string"
+ }
+ }
Review comment:
```suggestion
},
"additionalProperties": False,
"required": ["envName", "secretName"]
```
##########
File path: chart/values.schema.json
##########
@@ -89,13 +89,18 @@
"description": "Specify scheduling constraints for all pods.",
"type": "object",
"default": {},
- "x-docsSection": "Kubernetes"
+ "x-docsSection": "Kubernetes",
+ "$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0/_definitions.json#/definitions/io.k8s.api.core.v1.Affinity"
},
"tolerations": {
"description": "Specify Tolerations for all pods.",
"type": "array",
"default": [],
- "x-docsSection": "Kubernetes"
+ "x-docsSection": "Kubernetes",
+ "items": {
+ "type": "object",
+ "$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0/_definitions.json#/definitions/io.k8s.api.core.v1.Toleration"
Review comment:
Thoughts on using the strict variant? e.g:
https://github.com/yannh/kubernetes-json-schema/blob/master/v1.22.0-standalone-strict/_definitions.json
We are strict by default in our schema, I think it makes sense to continue that trend here too.
##########
File path: chart/values.schema.json
##########
@@ -89,13 +89,18 @@
"description": "Specify scheduling constraints for all pods.",
"type": "object",
"default": {},
- "x-docsSection": "Kubernetes"
+ "x-docsSection": "Kubernetes",
+ "$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0/_definitions.json#/definitions/io.k8s.api.core.v1.Affinity"
},
"tolerations": {
"description": "Specify Tolerations for all pods.",
"type": "array",
"default": [],
- "x-docsSection": "Kubernetes"
+ "x-docsSection": "Kubernetes",
+ "items": {
+ "type": "object",
+ "$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0/_definitions.json#/definitions/io.k8s.api.core.v1.Toleration"
Review comment:
Is there a benefit to using the full `_definitions.json` over using specific schemas? e.g:
https://github.com/yannh/kubernetes-json-schema/blob/master/v1.22.0-standalone-strict/toleration-v1.json
##########
File path: chart/values.schema.json
##########
@@ -2069,11 +2258,38 @@
"annotations": {
"description": "Annotations for the webserver Service.",
"type": "object",
- "default": {}
+ "default": {},
+ "additionalProperties": {
+ "type": "string"
+ }
},
"ports": {
"description": "Ports for the webserver Service.",
"type": "array",
+ "items": {
+ "type": "object",
+ "additionalProperties": false,
+ "properties": {
+ "name": {
+ "type": "string"
+ },
+ "port": {
+ "type": [
+ "string",
+ "integer"
+ ]
+ },
+ "targetPort": {
+ "type": [
+ "string",
+ "integer"
+ ]
+ },
+ "protocol": {
+ "type": "string"
+ }
+ }
+ },
Review comment:
We should be able to use `ServicePort` here (and with flower), no?
```suggestion
"items": {
"$ref": "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.22.0/_definitions.json#/definitions/io.k8s.api.core.v1.ServicePort"
},
```
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #19181: More strict schema for values.yaml
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #19181:
URL: https://github.com/apache/airflow/pull/19181#discussion_r741611007
##########
File path: chart/values.schema.json
##########
@@ -2069,11 +2258,38 @@
"annotations": {
"description": "Annotations for the webserver Service.",
"type": "object",
- "default": {}
+ "default": {},
+ "additionalProperties": {
+ "type": "string"
+ }
},
"ports": {
"description": "Ports for the webserver Service.",
"type": "array",
+ "items": {
+ "type": "object",
+ "additionalProperties": false,
+ "properties": {
+ "name": {
+ "type": "string"
+ },
+ "port": {
+ "type": [
+ "string",
+ "integer"
+ ]
+ },
+ "targetPort": {
+ "type": [
+ "string",
+ "integer"
+ ]
+ },
+ "protocol": {
+ "type": "string"
+ }
+ }
+ },
Review comment:
No. It is not compatible with current default value:
```
{
"name": "airflow-ui",
"port": "{{ .Values.ports.airflowUI }}"
}
```
In the Kubernetes schema, the `port` field is defined as `integer`, but we use `string`.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org