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

fix: read config data with bytes (python3) #86

Merged
merged 1 commit into from
Sep 19, 2018

Conversation

tomplus
Copy link
Member

@tomplus tomplus commented Sep 17, 2018

I came across a problem where kubeconfig has certificates stored as binary data.

apiVersion: v1
clusters:
- cluster:
    certificate-authority-data: !!binary |
      TAMwdExTSUNSVWRKVGlCRFJWOlVTVVpKUTBLVVJTMHRMUzB0A2sxSlNVWm1SRU5EUVRKVFowRjNT
      ............
      bZBKUkhZeFIzQnNARGRuUFQwS0xTMHFMUzFGVGtRZ1EwVlNWRWWHU1VOQlZFZXRMUzB1TFE9PQ==
...

Python 3 sees this as bytes and the encoding problem appears.

Traceback (most recent call last):
  File "examples/example1.py", line 33, in <module>
    main()
  File "examples/example1.py", line 22, in main
    config.load_kube_config()
  File "/tmp/python/kubernetes/config/kube_config.py", line 520, in load_kube_config
    loader.load_and_set(config)
  File "/tmp/python/kubernetes/config/kube_config.py", line 402, in load_and_set
    self._load_cluster_info()
  File "/tmp/python/kubernetes/config/kube_config.py", line 381, in _load_cluster_info
    file_base_path=self._config_base_path).as_file()
  File "/tmp/python/kubernetes/config/kube_config.py", line 104, in as_file
    base64.decodestring(self._data.encode()))
AttributeError: 'bytes' object has no attribute 'encode'

In this PR I've added a check if _data are the str type.

Thanks.

@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Sep 17, 2018
@@ -65,6 +65,9 @@ def _raise_exception(st):
TEST_DATA = "test-data"
TEST_DATA_BASE64 = _base64(TEST_DATA)

TEST_DATA_BYTES = b"test-data"
TEST_DATA_BASE64_BYTES = b"dGVzdC1kYXRh"
Copy link
Member

Choose a reason for hiding this comment

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

nit: you can use TEST_DATA.encode() or bytes(TEST_DATA, encoding='utf-8') instead of introducing new variables

Copy link
Member Author

Choose a reason for hiding this comment

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

+1, fixed

Copy link
Member

Choose a reason for hiding this comment

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

Thanks! Could you squash the commits?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@tomplus
Copy link
Member Author

tomplus commented Sep 19, 2018

/test all

@tomplus
Copy link
Member Author

tomplus commented Sep 19, 2018

I have no idea how to test it again. The previous build failed but it looks like an Travis' issue.

@roycaihw
Copy link
Member

/lgtm
/approve

Tests are green now. I manually re-triggered the tests. It would be great if we have the retest commends. Maybe worth a test-infra issue...

@k8s-ci-robot k8s-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 19, 2018
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: roycaihw, tomplus

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 19, 2018
@k8s-ci-robot k8s-ci-robot merged commit 7d1e449 into kubernetes-client:master Sep 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm Indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants