Add support for Context Profiles and Service Groups#535
Conversation
Add support to create/remove Context Profiles and Service Groups Signed-off-by: William Stoneman <william.stoneman@broadcom.com>
Updated get_custom_attributes_from_display_name function for profiles with no attributes
|
|
||
| EXAMPLES = ''' | ||
| - name: Create a new Security Service | ||
| nsxt_security_services: |
There was a problem hiding this comment.
It seems like you used another module as a template. There are quite a few places that need to be updated to reflect the unique attrs of this module. For example, this should show nsxt_context_profiles.
|
|
||
|
|
||
| def main(): | ||
| argument_spec = vmware_argument_spec() |
There was a problem hiding this comment.
Since this is an NSX Policy resource, you can use the template for nsxt_policy*.py modules. It removes a lot of boilerplate. Refer https://github.com/vmware/ansible-for-nsxt/blob/master/plugins/modules/nsxt_policy_ip_block.py or https://github.com/vmware/ansible-for-nsxt/blob/master/plugins/modules/nsxt_policy_segment.py
If you don't want to go that way, that's fine too
There was a problem hiding this comment.
Based on timeframes for my project, we will utilize this approach. I will look into changing it for a future release.
| if module.check_mode: | ||
| module.exit_json(changed=True, debug_out=str(json.dumps(context_params)), id='12345') | ||
|
|
||
| #print("?????????????????????????") |
| manager_url = 'https://{}/policy/api/v1'.format(mgr_hostname) | ||
|
|
||
| # err_msg = 'NSX v9.0.0 and above do not support MP resources in nsxt_ip_blocks.py. Please use nsxt_policy_ip_block.py module.' | ||
| # validate_nsx_mp_support(module, manager_url, mgr_username, mgr_password, validate_certs, err_msg) |
There was a problem hiding this comment.
we can remove this, ruight?
| context_params['display_name']) | ||
| if existing_services is None: | ||
| return False | ||
| if existing_services['attributes'] != context_params['attributes']: |
There was a problem hiding this comment.
are you certain this op is fine? what if list size is not the same or it's a nested dict?
There was a problem hiding this comment.
This has been tested. If the list of attributes differ it will "update" the profile.
There was a problem hiding this comment.
same comment as in other module: the user may want to update other attrs
There was a problem hiding this comment.
I took another look at it and updated the code to support display_name and description support. However, to allow this to work the end user needs to supply the ID. Otherwise there is no way to reference the object if they decide to update the display_name.
| } | ||
|
|
||
| try: | ||
| id = context_params['display_name'].replace(" ", "_") |
There was a problem hiding this comment.
afaik, id and display_name can be different. Other modules support that.
There was a problem hiding this comment.
It can be difference, but if the display name follows proper naming standards (no spaces and use of "_") then the ID will be the same. This approach just simplifies the deployment and usage of the objects.
There was a problem hiding this comment.
I think you don't have to specify the ID if display_name is present. If that's true, let server handle the behavior. Otherwise, you might end up creating a different behavior than the expectation
There was a problem hiding this comment.
even worse, what if the user updates display_name? You'll end up using wrong ID and it could fail?
There was a problem hiding this comment.
When creating the object you need to specify the ID in the URI. Having them the same simplifies the automation when calling the object in a different task.
If the end user updates the display_name later then it wont update the ID, this is definitely an issue with all nsxt objects.
There was a problem hiding this comment.
I took another look at it and updated the code to support display_name and description support. However, to allow this to work the end user needs to supply the ID. Otherwise there is no way to reference the object if they decide to update the display_name.
Updated COntext Profile based on comments made on PR
|
|
||
| EXAMPLES = ''' | ||
| - name: Create a new Context Profile | ||
| nsxt_security_services: |
| } | ||
|
|
||
| try: | ||
| id = context_params['display_name'].replace(" ", "_") |
There was a problem hiding this comment.
I think you don't have to specify the ID if display_name is present. If that's true, let server handle the behavior. Otherwise, you might end up creating a different behavior than the expectation
| def get_services_from_display_name(module, manager_url, mgr_username, mgr_password, validate_certs, display_name): | ||
| services = get_services(module, manager_url, mgr_username, mgr_password, validate_certs) | ||
| for service in services['results']: | ||
| if service.__contains__('display_name') and service['display_name'] == display_name: |
There was a problem hiding this comment.
consider
if 'display_name' in service and ...
There was a problem hiding this comment.
I took another look at it and updated the code to support display_name and description support. However, to allow this to work the end user needs to supply the ID. Otherwise there is no way to reference the object if they decide to update the display_name.
| service_params['display_name']) | ||
| if existing_services is None: | ||
| return False | ||
| if existing_services['service_entries'] != service_params['service_entries']: |
There was a problem hiding this comment.
what if the user wants to update anything else like description or display name?
There was a problem hiding this comment.
I took another look at it and updated the code to support display_name and description support. However, to allow this to work the end user needs to supply the ID. Otherwise there is no way to reference the object if they decide to update the display_name.
| context_params['display_name']) | ||
| if existing_services is None: | ||
| return False | ||
| if existing_services['attributes'] != context_params['attributes']: |
There was a problem hiding this comment.
same comment as in other module: the user may want to update other attrs
| } | ||
|
|
||
| try: | ||
| id = context_params['display_name'].replace(" ", "_") |
There was a problem hiding this comment.
even worse, what if the user updates display_name? You'll end up using wrong ID and it could fail?
Fixed update support to allow for display name and description update
|
I took another look at it and updated the code to support display_name and description support. However, to allow this to work the end user needs to supply the ID. Otherwise there is no way to reference the object if they decide to update the display_name. |
2 participants
Add support to create/remove Context Profiles and Service Groups
Signed-off-by: William Stoneman william.stoneman@broadcom.com