Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the Google Cloud SDK installation from a manual archive-based approach to using the official APT repository, and adds authentication configuration functionality.
- Replaced manual download/extract installation with APT package manager installation
- Added Google Cloud authentication using service account credentials
- Introduced new configuration templates and authentication tasks
Reviewed Changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| defaults/main.yml | Updated default variables for APT-based installation and added keyring configuration |
| tasks/main.yml | Completely rewritten to use APT repository installation and authentication |
| tasks/raw_install.yml | New file containing the original manual installation approach |
| tasks/authenticate.yml | New authentication task file with credential setup |
| vars/main.yml | Added credentials file variable |
| templates/default_config.j2 | New template for default gcloud configuration |
| files | Added symbolic link to shared files directory |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| mode: 0600 | ||
| - name: Authenticate with Google Cloud | ||
| ansible.builtin.shell: | ||
| cmd: "gcloud auth login --creds-file=/home/njl/.config/gcloud/{{ gcloud_credentials }}" |
There was a problem hiding this comment.
The flag --creds-file is incorrect. The correct flag for gcloud auth login with service account credentials is --cred-file (singular).
| cmd: "gcloud auth login --creds-file=/home/njl/.config/gcloud/{{ gcloud_credentials }}" | |
| cmd: "gcloud auth login --cred-file=/home/njl/.config/gcloud/{{ gcloud_credentials }}" |
| mode: "0600" | ||
| - name: Authenticate with Google Cloud | ||
| ansible.builtin.shell: | ||
| cmd: "gcloud auth login --cred-file=/home/njl/.config/gcloud/{{ gcloud_credentials }}" |
There was a problem hiding this comment.
For service account authentication, use gcloud auth activate-service-account --key-file instead of gcloud auth login --cred-file. The login command is for user authentication, not service accounts.
| cmd: "gcloud auth login --cred-file=/home/njl/.config/gcloud/{{ gcloud_credentials }}" | |
| cmd: "gcloud auth activate-service-account --key-file=/home/njl/.config/gcloud/{{ gcloud_credentials }}" |
| name: "/home/njl/.config/gcloud/" | ||
| state: directory | ||
| mode: "0700" | ||
| - name: Copy the archive from the Ansible controller | ||
| ansible.builtin.copy: | ||
| src: "{{ gcloud_credentials }}" | ||
| dest: "/home/njl/.config/gcloud/{{ gcloud_credentials }}" | ||
| mode: "0600" | ||
| - name: Authenticate with Google Cloud | ||
| ansible.builtin.shell: | ||
| cmd: "gcloud auth login --cred-file=/home/njl/.config/gcloud/{{ gcloud_credentials }}" |
There was a problem hiding this comment.
Hard-coded path '/home/njl' should be replaced with a variable like {{ ansible_env.HOME }} or {{ local_ssh_user_home }} to make the role more reusable.
| name: "/home/njl/.config/gcloud/" | |
| state: directory | |
| mode: "0700" | |
| - name: Copy the archive from the Ansible controller | |
| ansible.builtin.copy: | |
| src: "{{ gcloud_credentials }}" | |
| dest: "/home/njl/.config/gcloud/{{ gcloud_credentials }}" | |
| mode: "0600" | |
| - name: Authenticate with Google Cloud | |
| ansible.builtin.shell: | |
| cmd: "gcloud auth login --cred-file=/home/njl/.config/gcloud/{{ gcloud_credentials }}" | |
| name: "{{ ('~' + local_ssh_user + '/.config/gcloud/') | expanduser }}" | |
| state: directory | |
| mode: "0700" | |
| - name: Copy the archive from the Ansible controller | |
| ansible.builtin.copy: | |
| src: "{{ gcloud_credentials }}" | |
| dest: "{{ ('~' + local_ssh_user + '/.config/gcloud/' + gcloud_credentials) | expanduser }}" | |
| mode: "0600" | |
| - name: Authenticate with Google Cloud | |
| ansible.builtin.shell: | |
| cmd: "gcloud auth login --cred-file={{ ('~' + local_ssh_user + '/.config/gcloud/' + gcloud_credentials) | expanduser }}" |
| name: "/home/njl/.config/gcloud/" | ||
| state: directory | ||
| mode: "0700" | ||
| - name: Copy the archive from the Ansible controller | ||
| ansible.builtin.copy: | ||
| src: "{{ gcloud_credentials }}" | ||
| dest: "/home/njl/.config/gcloud/{{ gcloud_credentials }}" | ||
| mode: "0600" | ||
| - name: Authenticate with Google Cloud | ||
| ansible.builtin.shell: | ||
| cmd: "gcloud auth login --cred-file=/home/njl/.config/gcloud/{{ gcloud_credentials }}" |
There was a problem hiding this comment.
Hard-coded path '/home/njl' should be replaced with a variable like {{ ansible_env.HOME }} or {{ local_ssh_user_home }} to make the role more reusable.
| name: "/home/njl/.config/gcloud/" | |
| state: directory | |
| mode: "0700" | |
| - name: Copy the archive from the Ansible controller | |
| ansible.builtin.copy: | |
| src: "{{ gcloud_credentials }}" | |
| dest: "/home/njl/.config/gcloud/{{ gcloud_credentials }}" | |
| mode: "0600" | |
| - name: Authenticate with Google Cloud | |
| ansible.builtin.shell: | |
| cmd: "gcloud auth login --cred-file=/home/njl/.config/gcloud/{{ gcloud_credentials }}" | |
| name: "{{ local_ssh_user_home }}/.config/gcloud/" | |
| state: directory | |
| mode: "0700" | |
| - name: Copy the archive from the Ansible controller | |
| ansible.builtin.copy: | |
| src: "{{ gcloud_credentials }}" | |
| dest: "{{ local_ssh_user_home }}/.config/gcloud/{{ gcloud_credentials }}" | |
| mode: "0600" | |
| - name: Authenticate with Google Cloud | |
| ansible.builtin.shell: | |
| cmd: "gcloud auth login --cred-file={{ local_ssh_user_home }}/.config/gcloud/{{ gcloud_credentials }}" |
| dest: "/home/njl/.config/gcloud/{{ gcloud_credentials }}" | ||
| mode: 0600 | ||
| - name: Authenticate with Google Cloud | ||
| ansible.builtin.shell: | ||
| cmd: "gcloud auth login --creds-file=/home/njl/.config/gcloud/{{ gcloud_credentials }}" |
There was a problem hiding this comment.
Hard-coded path '/home/njl' should be replaced with a variable to make the authentication task reusable across different users.
| dest: "/home/njl/.config/gcloud/{{ gcloud_credentials }}" | |
| mode: 0600 | |
| - name: Authenticate with Google Cloud | |
| ansible.builtin.shell: | |
| cmd: "gcloud auth login --creds-file=/home/njl/.config/gcloud/{{ gcloud_credentials }}" | |
| dest: "{{ ansible_env.HOME }}/.config/gcloud/{{ gcloud_credentials }}" | |
| mode: 0600 | |
| - name: Authenticate with Google Cloud | |
| ansible.builtin.shell: | |
| cmd: "gcloud auth login --creds-file={{ ansible_env.HOME }}/.config/gcloud/{{ gcloud_credentials }}" |
| dest: "/home/njl/.config/gcloud/{{ gcloud_credentials }}" | ||
| mode: 0600 | ||
| - name: Authenticate with Google Cloud | ||
| ansible.builtin.shell: | ||
| cmd: "gcloud auth login --creds-file=/home/njl/.config/gcloud/{{ gcloud_credentials }}" |
There was a problem hiding this comment.
Hard-coded path '/home/njl' should be replaced with a variable to make the authentication task reusable across different users.
| dest: "/home/njl/.config/gcloud/{{ gcloud_credentials }}" | |
| mode: 0600 | |
| - name: Authenticate with Google Cloud | |
| ansible.builtin.shell: | |
| cmd: "gcloud auth login --creds-file=/home/njl/.config/gcloud/{{ gcloud_credentials }}" | |
| dest: "{{ ansible_env.HOME }}/.config/gcloud/{{ gcloud_credentials }}" | |
| mode: 0600 | |
| - name: Authenticate with Google Cloud | |
| ansible.builtin.shell: | |
| cmd: "gcloud auth login --creds-file={{ ansible_env.HOME }}/.config/gcloud/{{ gcloud_credentials }}" |
| account = nick.lange@gmail.com | ||
| project = zeta-environs-173316 |
There was a problem hiding this comment.
Hard-coded email and project values should be replaced with variables like {{ gcloud_account }} and {{ gcloud_project }} to make the template configurable.
| account = nick.lange@gmail.com | |
| project = zeta-environs-173316 | |
| account = {{ gcloud_account }} | |
| project = {{ gcloud_project }} |
| @@ -0,0 +1,86 @@ | |||
| --- | |||
| - block: | |||
| - name: Insure temp path exists | |||
There was a problem hiding this comment.
Spelling error: 'Insure' should be 'Ensure' in the task name.
| - name: Insure temp path exists | |
| - name: Ensure temp path exists |
No description provided.