Get defaults from CLI for GCE and OCI if not provided#87
Conversation
27b34ce to
1eb8f59
Compare
|
@lucasmoura Thanks for the tips. I've pushed a new version that uses the google credentials if provided, and also provides a default (the default for gcloud) that can be overridden. |
OddBloke
left a comment
There was a problem hiding this comment.
Thanks for this, James! I think your initial implementation of the Google stuff is closer to what we want: comments inline, of course.
| logging.getLogger('googleapiclient.discovery').setLevel(logging.WARNING) | ||
|
|
||
| DEFAULT_CREDENTIALS_PATH = Path.home() / '.config' / 'gcloud' / \ | ||
| 'application_default_credentials.json' |
There was a problem hiding this comment.
I'm not 100% sure that this is the correct path: it does exist on my desktop, but if I initialise a new gcloud inside a container (to the point where I can list my instances), I don't see it:
# tree .config/gcloud/
.config/gcloud/
├── access_tokens.db
├── active_config
├── cache
│ └── daniel.watkins@canonical.com
├── config_sentinel
├── configurations
│ └── config_default
├── credentials.db
├── gce
├── legacy_credentials
│ └── daniel.watkins@canonical.com
│ └── adc.json
└── logs
└── 2021.01.08
├── 17.27.25.441108.log
├── 17.27.37.404385.log
├── 17.27.41.272001.log
├── 17.28.11.613997.log
├── 17.28.16.418870.log
├── 17.28.24.243745.log
└── 17.28.26.915831.log
adc.json does look like the same file format (and adc is presumably an abbreviation for application_default_credentials?).
| if project is None: | ||
| with credentials_path.open() as f: | ||
| config = json.load(f) | ||
| if 'project_id' in config: |
There was a problem hiding this comment.
I'm not sure this is correct either; in both my desktop and in-container ADC file, I have only 4 keys: client_id, client_secret, refresh_token, and type.
When prompted to set my project in the container, it was set in .config/gcloud/configurations/config_default (which appears to be selected by default being in .config/gcloud/active_config).
(I believe that this file is used for authenticating your user to the Google APIs, which is orthogonal to the project being used: users have access to many projects.)
| except FileNotFoundError as e: | ||
| raise exception from e | ||
| if not result.ok: | ||
| raise exception |
There was a problem hiding this comment.
This is nit-level, so don't worry too much about it: will this include the output from the failing command? (That would save folks from having to re-run the command to see what the exact error was.)
|
Thanks for the review @OddBloke . In UAC, they don't install gcloud for their pycloudlib usage, so I thought I could cover multiple use cases this way, but it looks like file locations and keys don't stay consistent across versions. Since shelling out seems less likely to change than file/key locations, I'll go back to shelling out. If you don't have gcloud installed, the old way should still work. |
698946f to
e56278d
Compare
|
When running this in combination with canonical/cloud-init#757, I see the following error when I run Traceback (most recent call last):
File "/home/daniel/dev/cloud-init/tests/integration_tests/conftest.py", line 236, in class_client
with _client(request, fixture_utils, session_cloud) as client:
File "/usr/lib/python3.8/contextlib.py", line 113, in __enter__
return next(self.gen)
File "/home/daniel/dev/cloud-init/tests/integration_tests/conftest.py", line 210, in _client
with session_cloud.launch(
File "/home/daniel/dev/cloud-init/tests/integration_tests/clouds.py", line 165, in launch
pycloudlib_instance = self._perform_launch(kwargs)
File "/home/daniel/dev/cloud-init/tests/integration_tests/clouds.py", line 135, in _perform_launch
pycloudlib_instance = self.cloud_instance.launch(**launch_kwargs)
File "/home/daniel/dev/pycloudlib/pycloudlib/gce/cloud.py", line 206, in launch
operation = self.compute.instances().insert(
File "/home/daniel/.virtualenvs/cloud-init/lib/python3.8/site-packages/googleapiclient/_helpers.py", line 130, in positional_wrapper
return wrapped(*args, **kwargs)
File "/home/daniel/.virtualenvs/cloud-init/lib/python3.8/site-packages/googleapiclient/http.py", line 844, in execute
resp, content = _retry_request(
File "/home/daniel/.virtualenvs/cloud-init/lib/python3.8/site-packages/googleapiclient/http.py", line 164, in _retry_request
resp, content = http.request(uri, method, *args, **kwargs)
File "/home/daniel/.virtualenvs/cloud-init/lib/python3.8/site-packages/google_auth_httplib2.py", line 189, in request
self.credentials.before_request(
File "/home/daniel/.virtualenvs/cloud-init/lib/python3.8/site-packages/google/auth/credentials.py", line 133, in before_request
self.refresh(request)
File "/home/daniel/.virtualenvs/cloud-init/lib/python3.8/site-packages/google/oauth2/credentials.py", line 200, in refresh
access_token, refresh_token, expiry, grant_response = _client.refresh_grant(
File "/home/daniel/.virtualenvs/cloud-init/lib/python3.8/site-packages/google/oauth2/_client.py", line 248, in refresh_grant
response_data = _token_endpoint_request(request, token_uri, body)
File "/home/daniel/.virtualenvs/cloud-init/lib/python3.8/site-packages/google/oauth2/_client.py", line 124, in _token_endpoint_request
_handle_error_response(response_body)
File "/home/daniel/.virtualenvs/cloud-init/lib/python3.8/site-packages/google/oauth2/_client.py", line 60, in _handle_error_response
raise exceptions.RefreshError(error_details, response_body)
google.auth.exceptions.RefreshError: ('invalid_grant: Bad Request', '{\n "error": "invalid_grant",\n "error_description": "Bad Request"\n}')Earlier in the run, I see: And in the warnings summary, I see: After running that command, not only do the tests run successfully, but I both have a So perhaps what we actually need to do is check for |
|
@OddBloke I don't think we provide guidance for setting up any cloud CLIs, do we? We kind of leave that as an exercise for the user. Since you were given a warning telling you what you need to do, I'm thinking we don't need to do anything else atm. While I'm not against giving guidance for setting up the various CLIs, I think that's beyond the scope of this particular PR. Thoughts? |
Stop requiring to pass in project for gce and compartment for oci