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

Fix apiserver_id 'get' method #184

Merged
merged 1 commit into from
Feb 21, 2020
Merged

Fix apiserver_id 'get' method #184

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
9 changes: 6 additions & 3 deletions config/kube_config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -326,9 +326,12 @@ def _refresh_azure_token(self, config):
)
refresh_token = config['refresh-token']
client_id = config['client-id']
apiserver_id = config.get(
'apiserver-id',
'00000002-0000-0000-c000-000000000000')
apiserver_id = '00000002-0000-0000-c000-000000000000'
try:
apiserver_id = config['apiserver-id']
except ConfigException:
# We've already set a default above
pass
token_response = context.acquire_token_with_refresh_token(
refresh_token, client_id, apiserver_id)

Expand Down
63 changes: 0 additions & 63 deletions config/kube_config_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -460,20 +460,6 @@ class TestKubeConfigLoader(BaseTestCase):
"user": "azure_str_error"
}
},
{
"name": "azure_no_apiserver",
"context": {
"cluster": "default",
"user": "azure_no_apiserver"
}
},
{
"name": "azure_bad_apiserver",
"context": {
"cluster": "default",
"user": "azure_bad_apiserver"
}
},
{
"name": "expired_oidc",
"context": {
Expand Down Expand Up @@ -771,39 +757,6 @@ class TestKubeConfigLoader(BaseTestCase):
}
}
},
{
"name": "azure_no_apiserver",
"user": {
"auth-provider": {
"config": {
"access-token": TEST_AZURE_TOKEN,
"environment": "AzurePublicCloud",
"expires-in": "0",
"expires-on": "156207275",
"refresh-token": "refreshToken",
"tenant-id": "9d2ac018-e843-4e14-9e2b-4e0ddac75433"
},
"name": "azure"
}
}
},
{
"name": "azure_bad_apiserver",
"user": {
"auth-provider": {
"config": {
"access-token": TEST_AZURE_TOKEN,
"apiserver-id": "ApiserverId",
"environment": "AzurePublicCloud",
"expires-in": "0",
"expires-on": "156207275",
"refresh-token": "refreshToken",
"tenant-id": "9d2ac018-e843-4e14-9e2b-4e0ddac75433"
},
"name": "azure"
}
}
},
{
"name": "expired_oidc",
"user": {
Expand Down Expand Up @@ -1161,22 +1114,6 @@ def test_azure_with_expired_int_error(self):
provider = loader._user['auth-provider']
self.assertRaises(ValueError, loader._azure_is_expired, provider)

def test_azure_with_no_apiserver(self):
loader = KubeConfigLoader(
config_dict=self.TEST_KUBE_CONFIG,
active_context="azure_no_apiserver",
)
provider = loader._user['auth-provider']
self.assertTrue(loader._azure_is_expired(provider))

def test_azure_with_bad_apiserver(self):
Copy link
Member

Choose a reason for hiding this comment

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

The code fix looks good to me. Could you point out which other test cases cover these pruned cases?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These two tests are calling the same function as test_azure_with_expired_num with the same arguments. The apiserver values here aren't actually used in _azure_is_expired and _refresh_azure_token is not currently covered by tests (which is why this slipped through in the first place, and the test coverage didn't really change)
https://github.com/kubernetes-client/python-base/pull/184/files#diff-3123bf09ac4950ea8bf8de46fb38eb83L1132

loader = KubeConfigLoader(
config_dict=self.TEST_KUBE_CONFIG,
active_context="azure_bad_apiserver",
)
provider = loader._user['auth-provider']
self.assertTrue(loader._azure_is_expired(provider))

def test_user_pass(self):
expected = FakeConfig(host=TEST_HOST, token=TEST_BASIC_TOKEN)
actual = FakeConfig()
Expand Down