Skip to content

Commit

Permalink
azure_rm_aks: support system-assigned (managed) identity, (#514)
Browse files Browse the repository at this point in the history
* azure_rm_aks: make linux_profile and service_principal optional, support system-assigned (managed) identity

* azure_rm_aks: adjust docs formatting

Co-authored-by: Fred-sun <37327967+Fred-sun@users.noreply.github.com>

* azure_rm_aks: add a test for the minimal parameters cluster definition

* azure_rm_aks: fix sanity-checks / pep8 requirements

* Add instructions for tests / sanity checks

Co-authored-by: Fred-sun <37327967+Fred-sun@users.noreply.github.com>
  • Loading branch information
geekq and Fred-sun authored Jul 22, 2021
1 parent 235c564 commit bd40c1d
Show file tree
Hide file tree
Showing 4 changed files with 117 additions and 11 deletions.
7 changes: 7 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,13 @@ When contributing to this repository, please first discuss the change you wish t
1. Create a branch based on the latest `dev` branch, commit your changes on this branch.
1. You may merge the Pull Request in once you have the sign-off of two other developers, or if you do not have permission to do that, you may request the second reviewer to merge it for you.

## Tests / sanity checks

1. Please provide integration tests showing the changed behavior/functionality under `tests/integration/targets/<relevant-module>/tasks`.
1. Think about updating the documentation and examples for the changed module.
1. Please run a sanity check. Install prerequisites `pip install -r sanity-requirements-azure.txt`, run with `ansible-test sanity --color -v --junit`. Read more at https://docs.ansible.com/ansible/latest/dev_guide/testing_sanity.html.
1. There is a script `tests/utils/ado/ado.sh` for running tests inside an Azure DevOps pipeline. Unfortunately the pipeline and results are not visible for the public. You can perhaps adapt the parts of the script or use a small playbook to run the task list of the integration tests mentioned above.

## Release Process

1. Create a release branch from the target commit on dev branch.
Expand Down
46 changes: 36 additions & 10 deletions plugins/modules/azure_rm_aks.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
linux_profile:
description:
- The Linux profile suboptions.
- Optional, provide if you need an ssh access to the cluster nodes.
suboptions:
admin_username:
description:
Expand Down Expand Up @@ -126,7 +127,7 @@
- 3
service_principal:
description:
- The service principal suboptions.
- The service principal suboptions. If not provided - use system-assigned managed identity.
suboptions:
client_id:
description:
Expand Down Expand Up @@ -318,6 +319,17 @@
tags:
Environment: Production
- name: Use minimal parameters and system-assigned identity
azure_rm_aks:
name: myMinimalCluster
location: eastus
resource_group: myExistingResourceGroup
dns_prefix: akstest
agent_pool_profiles:
- name: default
count: 1
vm_size: Standard_D2_v2
- name: Remove a managed Azure Container Services (AKS) instance
azure_rm_aks:
name: myAKS
Expand Down Expand Up @@ -431,10 +443,13 @@ def create_linux_profile_dict(linuxprofile):
:param: linuxprofile: ContainerServiceLinuxProfile with the Azure callback object
:return: dict with the state on Azure
'''
return dict(
ssh_key=linuxprofile.ssh.public_keys[0].key_data,
admin_username=linuxprofile.admin_username
)
if linuxprofile:
return dict(
ssh_key=linuxprofile.ssh.public_keys[0].key_data,
admin_username=linuxprofile.admin_username
)
else:
return None


def create_service_principal_profile_dict(serviceprincipalprofile):
Expand Down Expand Up @@ -629,7 +644,7 @@ def __init__(self):

required_if = [
('state', 'present', [
'dns_prefix', 'linux_profile', 'agent_pool_profiles', 'service_principal'])
'dns_prefix', 'agent_pool_profiles'])
]

self.results = dict(changed=False)
Expand Down Expand Up @@ -680,7 +695,7 @@ def is_property_changed(profile, property, ignore_case=False):
return base != new

# Cannot Update the SSH Key for now // Let service to handle it
if is_property_changed('linux_profile', 'ssh_key'):
if self.linux_profile and is_property_changed('linux_profile', 'ssh_key'):
self.log(("Linux Profile Diff SSH, Was {0} / Now {1}"
.format(response['linux_profile']['ssh_key'], self.linux_profile.get('ssh_key'))))
to_be_updated = True
Expand All @@ -689,7 +704,7 @@ def is_property_changed(profile, property, ignore_case=False):
# self.log("linux_profile response : {0}".format(response['linux_profile'].get('admin_username')))
# self.log("linux_profile self : {0}".format(self.linux_profile[0].get('admin_username')))
# Cannot Update the Username for now // Let service to handle it
if is_property_changed('linux_profile', 'admin_username'):
if self.linux_profile and is_property_changed('linux_profile', 'admin_username'):
self.log(("Linux Profile Diff User, Was {0} / Now {1}"
.format(response['linux_profile']['admin_username'], self.linux_profile.get('admin_username'))))
to_be_updated = True
Expand Down Expand Up @@ -849,7 +864,17 @@ def create_update_aks(self):
if self.agent_pool_profiles:
agentpools = [self.create_agent_pool_profile_instance(profile) for profile in self.agent_pool_profiles]

service_principal_profile = self.create_service_principal_profile_instance(self.service_principal)
if self.service_principal:
service_principal_profile = self.create_service_principal_profile_instance(self.service_principal)
identity = None
else:
service_principal_profile = None
identity = self.managedcluster_models.ManagedClusterIdentity(type='SystemAssigned')

if self.linux_profile:
linux_profile = self.create_linux_profile_instance(self.linux_profile)
else:
linux_profile = None

parameters = self.managedcluster_models.ManagedCluster(
location=self.location,
Expand All @@ -858,7 +883,8 @@ def create_update_aks(self):
tags=self.tags,
service_principal_profile=service_principal_profile,
agent_pool_profiles=agentpools,
linux_profile=self.create_linux_profile_instance(self.linux_profile),
linux_profile=linux_profile,
identity=identity,
enable_rbac=self.enable_rbac,
network_profile=self.create_network_profile_instance(self.network_profile),
aad_profile=self.create_aad_profile_instance(self.aad_profile),
Expand Down
2 changes: 1 addition & 1 deletion requirements-azure.txt
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ azure-mgmt-trafficmanager==0.50.0
azure-mgmt-web==0.41.0
azure-nspkg==2.0.0
azure-storage==0.35.1
msrest==0.6.10
msrest==0.6.21
msrestazure==0.6.4
azure-keyvault==1.0.0a1
azure-graphrbac==0.61.1
Expand Down
73 changes: 73 additions & 0 deletions tests/integration/targets/azure_rm_aks/tasks/minimal-cluster.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
- set_fact:
rpfx: "{{ resource_group | hash('md5') | truncate(8, True, '') }}"
noderpfx: "{{ resource_group | hash('md5') | truncate(4, True, '') }}"

- name: Use minimal parameters and system-assigned identity
azure_rm_aks:
name: "minimal{{ rpfx }}"
location: eastus
resource_group: "{{ resource_group }}"
dns_prefix: "aks{{ rpfx }}"
agent_pool_profiles:
- name: default
count: 1
vm_size: Standard_DS1_v2
register: output

- name: Assert the AKS instance is well created
assert:
that:
- output.changed
- output.provisioning_state == 'Succeeded'

- name: Get AKS fact
azure_rm_aks_info:
name: "minimal{{ rpfx }}"
resource_group: "{{ resource_group }}"
register: fact

- name: Assert fact returns the created one
assert:
that:
- "fact.aks | length == 1"
- fact.aks[0].id == output.id

- name: Use minimal parameters and system-assigned identity (idempotent)
azure_rm_aks:
name: "minimal{{ rpfx }}"
location: eastus
resource_group: "{{ resource_group }}"
dns_prefix: "aks{{ rpfx }}"
agent_pool_profiles:
- name: default
count: 1
vm_size: Standard_DS1_v2
register: output

- name: Assert idempotent
assert:
that:
- not output.changed

- name: Delete the AKS instance
azure_rm_aks:
name: "minimal{{ rpfx }}"
resource_group: "{{ resource_group }}"
state: absent
register: output

- name: Assert the AKS instance is well deleted
assert:
that:
- output.changed

- name: Get AKS fact
azure_rm_aks_info:
name: "minimal{{ rpfx }}"
resource_group: "{{ resource_group }}"
register: fact

- name: Assert fact returns empty
assert:
that:
- "fact.aks | length == 0"

0 comments on commit bd40c1d

Please sign in to comment.