Skip to content
This repository has been archived by the owner on Mar 13, 2022. It is now read-only.

Support false values in configuration file #161

Merged
merged 1 commit into from
Sep 24, 2019
Merged

Support false values in configuration file #161

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion config/kube_config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -499,7 +499,7 @@ def safe_get(self, key):

def __getitem__(self, key):
v = self.safe_get(key)
if not v:
if v is None:
raise ConfigException(
'Invalid kube-config file. Expected key %s in %s'
% (key, self.name))
Expand Down
6 changes: 4 additions & 2 deletions config/kube_config_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -564,13 +564,14 @@ class TestKubeConfigLoader(BaseTestCase):
"server": TEST_SSL_HOST,
"certificate-authority-data":
TEST_CERTIFICATE_AUTH_BASE64,
"insecure-skip-tls-verify": False,
Copy link
Member

Choose a reason for hiding this comment

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

s/False/"false" in consistent with the test case below

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@roycaihw Thanks for the suggestion! I've just tried to replace False with "false" and faced an issue - the unit test fails. I think the cause is in how "insecure-skip-tls-verify" property is handled in kube_config.py (https://github.com/kubernetes-client/python-base/blob/master/config/kube_config.py#L443). The code by the link casts a string to a boolean. Both "true" and "false" are casted to True. What do you think if I replace "true" with True in the older test?

Copy link
Member

Choose a reason for hiding this comment

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

"true" and "false" are the original values in kubeconfig yaml files (e.g. insecure-skip-tls-verify: false). The test cases are testing values stored in the KubeConfigLoader object.

Could you verify what's the behavior in the yaml loader? Does it load "false" into a boolean False, or does it keep the string value "false"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I verified the behavior on the configuration file which has the following lines:

clusters:
- cluster:
    insecure-skip-tls-verify: false

The following code snippet was used for the verification:

import yaml

with open(<PATH_TO_CONFIG>) as f:
    config = yaml.safe_load(f)
    print config (**breakpoint was set here**)

Debug showed the following value for the property:
'insecure-skip-tls-verify' (4523129968) = {bool} False

Copy link
Member

Choose a reason for hiding this comment

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

Thanks. Let's use True and False in the test cases then

}
},
{
"name": "no_ssl_verification",
"cluster": {
"server": TEST_SSL_HOST,
"insecure-skip-tls-verify": "true",
"insecure-skip-tls-verify": True,
}
},
],
Expand Down Expand Up @@ -1076,7 +1077,8 @@ def test_ssl(self):
token=BEARER_TOKEN_FORMAT % TEST_DATA_BASE64,
cert_file=self._create_temp_file(TEST_CLIENT_CERT),
key_file=self._create_temp_file(TEST_CLIENT_KEY),
ssl_ca_cert=self._create_temp_file(TEST_CERTIFICATE_AUTH)
ssl_ca_cert=self._create_temp_file(TEST_CERTIFICATE_AUTH),
verify_ssl=True
)
actual = FakeConfig()
KubeConfigLoader(
Expand Down