Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add nodePort declaration to chart/values.schema.json #26945

Merged
merged 8 commits into from
Nov 14, 2022
Merged

Add nodePort declaration to chart/values.schema.json #26945

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
278e27a
Add nodePort declaration to chart/values.schema.json to allow explici…
Oct 7, 2022
773fe00
Add test for NodePort service type for webserver component
Oct 9, 2022
fb27f0f
Add NodePort test case where no NodePort is provided but service is s…
Oct 10, 2022
b8e2dc3
Add missing comma to test_nodeport_service parameters for style purposes
Oct 18, 2022
90606c7
Update test_nodeport_service unit test with new pytest format
Oct 21, 2022
6f1b237
Black formatting for tests/charts/test_webserver.py
Oct 21, 2022
892a43a
Remove unnecessary parentheses from nodeport test
joseph-max-coalfire Nov 8, 2022
92c07ee
Remove "name" value from nodeport test
joseph-max-coalfire Nov 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions chart/values.schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3465,6 +3465,12 @@
"integer"
]
},
"nodePort": {
"type": [
"string",
"integer"
]
},
"protocol": {
"type": "string"
}
Expand Down
29 changes: 29 additions & 0 deletions tests/charts/test_webserver.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -719,6 +719,35 @@ def test_should_add_component_specific_labels(self):
assert "test_label" in jmespath.search("metadata.labels", docs[0])
assert jmespath.search("metadata.labels", docs[0])["test_label"] == "test_label_value"

@pytest.mark.parametrize(
"ports, expected_ports",
[
(
[{"nodePort": "31000", "port": "8080"}],
[{"nodePort": 31000, "port": 8080}],
),
(
[{"port": "8080"}],
[{"port": 8080}],
),
],
)
def test_nodeport_service(self, ports, expected_ports):
Copy link
Contributor

@dstandish dstandish Oct 10, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

looks like your params aren't consistent with those provided in expand
but let's see what the tests tell us
i'd also add an entry (in expand) for when no nodePort is provided

Copy link
Contributor Author

@joseph-max-coalfire joseph-max-coalfire Oct 10, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

looks like your params aren't consistent with those provided in expand

@dstandish Not sure I follow-- I used the test_ports_overrides param structure as a reference, and it looks like CI checks are passing

i'd also add an entry (in expand) for when no nodePort is provided

Added (lines 730-733) and tested locally with success. We'll see if the remote CI checks pass as well

Copy link
Contributor

Choose a reason for hiding this comment

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

all good.

one thing though.

looking here, it looks like name is not a valid attr for nodeport. probably best to remov that.

docs = render_chart(
values={
"webserver": {
"service": {
"type": "NodePort",
"ports": ports,
}
},
},
show_only=["templates/webserver/webserver-service.yaml"],
)

assert "NodePort" == jmespath.search("spec.type", docs[0])
assert expected_ports == jmespath.search("spec.ports", docs[0])


class TestWebserverConfigmap:
def test_no_webserver_config_configmap_by_default(self):
Expand Down