jhg03a commented on a change in pull request #5685:
URL: https://github.com/apache/trafficcontrol/pull/5685#discussion_r608742557
##########
File path: infrastructure/ansible/roles/dataset_loader/defaults/main.yml
##########
@@ -108,6 +108,11 @@ dl_ds_default_users:
email: [email protected]
fullName: A local account with RO rights
role: read-only
+ - username: '{{ federation_user }}'
+ password: '{{ to_admin_user_password }}'
Review comment:
The password field here should probably just be removed since it's not
one anyone should ever login with. That will default it to a random 32
character string.
##########
File path: infrastructure/ansible/roles/dataset_loader/defaults/main.yml
##########
@@ -108,6 +108,11 @@ dl_ds_default_users:
email: [email protected]
fullName: A local account with RO rights
role: read-only
+ - username: '{{ federation_user }}'
+ password: '{{ to_admin_user_password }}'
+ email: '{{ federation_user }@kabletown.invalid'
+ fullName: A local account with admin rights
+ role: admin
Review comment:
For a federation user, is admin level permissions required?
##########
File path: infrastructure/ansible/roles/dataset_loader/defaults/main.yml
##########
@@ -4042,3 +4051,16 @@ dl_ds_default_ds_template:
longDesc: A basic HTTP routed Delivery Service with Anonymous IP Block
Enabled
anonymousBlockingEnabled: true
uniqueKey: simple-http-anon-block
+
+# Federations
+dl_ds_merged_federations: "{{ dl_ds_default_federations }}"
+dl_ds_default_federations:
+ - deliveryService: simple-dns-for-federation-{{ Target_cdn_delegation |
lower }}
Review comment:
The var used here appears to be `federation` not `Target_cdn_delegation`
##########
File path:
infrastructure/ansible/roles/dataset_loader/tasks/federation_loader.yml
##########
@@ -0,0 +1,85 @@
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+- name: Create Federation
+ vars:
+ cdn_name_query: 'response[?xmlId == `{{ federation.deliveryService |
to_json }}`].cdnName | [0]'
+ cdn_name: '{{ get_all_ds.json | json_query(cdn_name_query) }}'
+ uri:
+ url: "{{ dl_to_url }}/api/{{ dl_to_api_version }}/cdns/{{ cdn_name
}}/federations"
+ method: POST
+ body: '{"cname":{{ federation.mappings.cname | to_json }},"ttl":{{
federation.mappings.ttl }}}'
Review comment:
This breaks convention of using j2 templates when bodies include
int/float/bool data types. If it works, that's fine.
##########
File path:
infrastructure/ansible/roles/dataset_loader/tasks/federation_loader.yml
##########
@@ -0,0 +1,85 @@
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+- name: Create Federation
+ vars:
+ cdn_name_query: 'response[?xmlId == `{{ federation.deliveryService |
to_json }}`].cdnName | [0]'
+ cdn_name: '{{ get_all_ds.json | json_query(cdn_name_query) }}'
+ uri:
+ url: "{{ dl_to_url }}/api/{{ dl_to_api_version }}/cdns/{{ cdn_name
}}/federations"
+ method: POST
+ body: '{"cname":{{ federation.mappings.cname | to_json }},"ttl":{{
federation.mappings.ttl }}}'
Review comment:
I don't believe it's necessary to call `to_json` on a single string
variable.
##########
File path:
infrastructure/ansible/roles/dataset_loader/tasks/federation_loader.yml
##########
@@ -0,0 +1,85 @@
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+- name: Create Federation
+ vars:
+ cdn_name_query: 'response[?xmlId == `{{ federation.deliveryService |
to_json }}`].cdnName | [0]'
+ cdn_name: '{{ get_all_ds.json | json_query(cdn_name_query) }}'
+ uri:
+ url: "{{ dl_to_url }}/api/{{ dl_to_api_version }}/cdns/{{ cdn_name
}}/federations"
+ method: POST
+ body: '{"cname":{{ federation.mappings.cname | to_json }},"ttl":{{
federation.mappings.ttl }}}'
+ register: create_federation_out
+
+- name: Get Federation ID
+ vars:
+ federation_id_query: response.id
+ set_fact:
+ federation_id: "{{ create_federation_out.json |
json_query(federation_id_query) }}"
+
+- name: Assign User to Federation
+ vars:
+ federation_user_query: "response[?username == `{{ federation_user }}`].id
| [0]"
+ federation_user_id: "{{ get_all_users.json |
json_query(federation_user_query) }}"
+ uri:
+ url: "{{ dl_to_url }}/api/{{ dl_to_api_version }}/federations/{{
federation_id }}/users"
+ method: POST
+ body: '{"userIds":[{{ federation_user_id }}],"replace":false}'
+
+- name: Assign Delivery Service to Federation
+ vars:
+ federation_ds_id_query: 'response[?xmlId == `{{ federation.deliveryService
| to_json }}`].id | [0]'
Review comment:
I think `to_json` is redundant here since it's just a simple string.
##########
File path:
infrastructure/ansible/roles/dataset_loader/tasks/federation_loader.yml
##########
@@ -0,0 +1,85 @@
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+- name: Create Federation
+ vars:
+ cdn_name_query: 'response[?xmlId == `{{ federation.deliveryService |
to_json }}`].cdnName | [0]'
+ cdn_name: '{{ get_all_ds.json | json_query(cdn_name_query) }}'
+ uri:
+ url: "{{ dl_to_url }}/api/{{ dl_to_api_version }}/cdns/{{ cdn_name
}}/federations"
+ method: POST
+ body: '{"cname":{{ federation.mappings.cname | to_json }},"ttl":{{
federation.mappings.ttl }}}'
+ register: create_federation_out
+
+- name: Get Federation ID
+ vars:
+ federation_id_query: response.id
+ set_fact:
+ federation_id: "{{ create_federation_out.json |
json_query(federation_id_query) }}"
+
+- name: Assign User to Federation
+ vars:
+ federation_user_query: "response[?username == `{{ federation_user }}`].id
| [0]"
+ federation_user_id: "{{ get_all_users.json |
json_query(federation_user_query) }}"
+ uri:
+ url: "{{ dl_to_url }}/api/{{ dl_to_api_version }}/federations/{{
federation_id }}/users"
+ method: POST
+ body: '{"userIds":[{{ federation_user_id }}],"replace":false}'
+
+- name: Assign Delivery Service to Federation
+ vars:
+ federation_ds_id_query: 'response[?xmlId == `{{ federation.deliveryService
| to_json }}`].id | [0]'
+ federation_ds_id: "{{ get_all_ds.json | json_query(federation_ds_id_query)
}}"
+ uri:
+ url: "{{ dl_to_url }}/api/{{ dl_to_api_version }}/federations/{{
federation_id }}/deliveryservices"
Review comment:
I think you meant `federation_ds_id`
##########
File path:
infrastructure/ansible/roles/dataset_loader/tasks/federation_loader.yml
##########
@@ -0,0 +1,85 @@
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+- name: Create Federation
+ vars:
+ cdn_name_query: 'response[?xmlId == `{{ federation.deliveryService |
to_json }}`].cdnName | [0]'
+ cdn_name: '{{ get_all_ds.json | json_query(cdn_name_query) }}'
+ uri:
+ url: "{{ dl_to_url }}/api/{{ dl_to_api_version }}/cdns/{{ cdn_name
}}/federations"
+ method: POST
+ body: '{"cname":{{ federation.mappings.cname | to_json }},"ttl":{{
federation.mappings.ttl }}}'
+ register: create_federation_out
+
+- name: Get Federation ID
+ vars:
+ federation_id_query: response.id
+ set_fact:
+ federation_id: "{{ create_federation_out.json |
json_query(federation_id_query) }}"
+
+- name: Assign User to Federation
+ vars:
+ federation_user_query: "response[?username == `{{ federation_user }}`].id
| [0]"
+ federation_user_id: "{{ get_all_users.json |
json_query(federation_user_query) }}"
+ uri:
+ url: "{{ dl_to_url }}/api/{{ dl_to_api_version }}/federations/{{
federation_id }}/users"
+ method: POST
+ body: '{"userIds":[{{ federation_user_id }}],"replace":false}'
+
+- name: Assign Delivery Service to Federation
+ vars:
+ federation_ds_id_query: 'response[?xmlId == `{{ federation.deliveryService
| to_json }}`].id | [0]'
+ federation_ds_id: "{{ get_all_ds.json | json_query(federation_ds_id_query)
}}"
+ uri:
+ url: "{{ dl_to_url }}/api/{{ dl_to_api_version }}/federations/{{
federation_id }}/deliveryservices"
+ method: POST
+ body: '{"dsIds":[{{ federation_ds_id }}],"replace":false}'
+
+- name: Create IPv4 Federation Resolvers
+ with_items: "{{ federation.mappings.resolve4 }}"
+ vars:
+ type_name: RESOLVE4
+ type_query: "[?name == `{{ type_name }}`].id | [0]"
Review comment:
If you only use a variable in one place with no potential to override in
some way, it shouldn't exist.
##########
File path: infrastructure/ansible/roles/dataset_loader/tasks/dataset_loader.yml
##########
@@ -838,6 +844,12 @@
method: GET
register: get_all_ds
+- name: Create Federations
+ include_tasks: federation_loader.yml
+ with_items: "{{ dl_ds_merged_federations }}"
Review comment:
Instead of looping on the federations, It needs to loop on the
`cdnDelegationList`.
##########
File path: infrastructure/ansible/roles/dataset_loader/defaults/main.yml
##########
@@ -108,6 +108,11 @@ dl_ds_default_users:
email: [email protected]
fullName: A local account with RO rights
role: read-only
+ - username: '{{ federation_user }}'
Review comment:
This var needs a default value such as using something higher up like
`dl_ds_default_federation_user`. That way you don't have to apply defaults on
every usage.
##########
File path:
infrastructure/ansible/roles/dataset_loader/tasks/federation_loader.yml
##########
@@ -0,0 +1,85 @@
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+- name: Create Federation
+ vars:
+ cdn_name_query: 'response[?xmlId == `{{ federation.deliveryService |
to_json }}`].cdnName | [0]'
+ cdn_name: '{{ get_all_ds.json | json_query(cdn_name_query) }}'
+ uri:
+ url: "{{ dl_to_url }}/api/{{ dl_to_api_version }}/cdns/{{ cdn_name
}}/federations"
+ method: POST
+ body: '{"cname":{{ federation.mappings.cname | to_json }},"ttl":{{
federation.mappings.ttl }}}'
+ register: create_federation_out
+
+- name: Get Federation ID
+ vars:
+ federation_id_query: response.id
+ set_fact:
+ federation_id: "{{ create_federation_out.json |
json_query(federation_id_query) }}"
+
+- name: Assign User to Federation
+ vars:
+ federation_user_query: "response[?username == `{{ federation_user }}`].id
| [0]"
+ federation_user_id: "{{ get_all_users.json |
json_query(federation_user_query) }}"
+ uri:
+ url: "{{ dl_to_url }}/api/{{ dl_to_api_version }}/federations/{{
federation_id }}/users"
+ method: POST
+ body: '{"userIds":[{{ federation_user_id }}],"replace":false}'
+
+- name: Assign Delivery Service to Federation
+ vars:
+ federation_ds_id_query: 'response[?xmlId == `{{ federation.deliveryService
| to_json }}`].id | [0]'
+ federation_ds_id: "{{ get_all_ds.json | json_query(federation_ds_id_query)
}}"
+ uri:
+ url: "{{ dl_to_url }}/api/{{ dl_to_api_version }}/federations/{{
federation_id }}/deliveryservices"
+ method: POST
+ body: '{"dsIds":[{{ federation_ds_id }}],"replace":false}'
+
+- name: Create IPv4 Federation Resolvers
+ with_items: "{{ federation.mappings.resolve4 }}"
+ vars:
+ type_name: RESOLVE4
+ type_query: "[?name == `{{ type_name }}`].id | [0]"
+ resolve4_type_id: "{{ get_all_types.json.response | json_query(type_query)
}}"
+ uri:
+ url: "{{ dl_to_url }}/api/{{ dl_to_api_version }}/federation_resolvers"
+ method: POST
+ body: '{"ipAddress":{{ item | to_json }},"typeId":{{ resolve4_type_id }}}'
+ register: ipv4_federation_resolver_out
+
+- name: Create IPv6 Federation Resolvers
+ with_items: "{{ federation.mappings.resolve6 }}"
+ vars:
+ type_name: RESOLVE6
+ type_query: "[?name == `{{ type_name }}`].id | [0]"
Review comment:
If you only use a variable in one place with no potential to override in
some way, it shouldn't exist.
##########
File path: infrastructure/ansible/roles/dataset_loader/defaults/main.yml
##########
@@ -108,6 +108,11 @@ dl_ds_default_users:
email: [email protected]
fullName: A local account with RO rights
role: read-only
+ - username: '{{ federation_user }}'
+ password: '{{ to_admin_user_password }}'
+ email: '{{ federation_user }@kabletown.invalid'
Review comment:
Missing curly brace
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
