Skip to content

Commit

Permalink
Support authentication in Bifrost
Browse files Browse the repository at this point in the history
* Switch from python-ironic-inspector-client to openstacksdk in
  ironic-inspector-rules. This allows us to use clouds.yaml to provide
  credentials.
* Enable authentication in Bifrost. Passwords are auto-generated by
  Bifrost, and stored files in /root/.config/bifrost/. This change
  depends on a Kolla Ansible patch that ensures that these credentials
  are persisted between recreations of the bifrost container.
* Copy clouds.yaml and (if present) a CA certificate from the Bifrost
  container to the seed host, under the Kayobe Ansible user (stack).
  This allows us to use the credentials to register introspection rules.
* This patch is needed by a Kolla Ansible patch that enables TLS in
  Bifrost, since we need the CA certificate on the host to register
  introspection rules when TLS is enabled.

Depends-On: https://review.opendev.org/c/openstack/kolla-ansible/+/851837
Needed-By: https://review.opendev.org/c/openstack/kolla-ansible/+/851838

Story: 2010206
Task: 45930

Change-Id: I757f1bb72afb01a4f1689bed292f5b71b9048fa0
(cherry picked from commit 32a82ea)
  • Loading branch information
markgoddard committed Nov 3, 2022
1 parent 5324851 commit 0ff1f8c
Show file tree
Hide file tree
Showing 11 changed files with 88 additions and 63 deletions.
1 change: 0 additions & 1 deletion ansible/overcloud-provision.yml
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,6 @@
bash -c '
export OS_CLOUD=bifrost &&
export BIFROST_INVENTORY_SOURCE=ironic &&
export OS_BAREMETAL_API_VERSION=1.34 &&
ansible-playbook -vvvv
/bifrost/playbooks/deploy-dynamic.yaml
--inventory /etc/bifrost/inventory/
Expand Down
3 changes: 1 addition & 2 deletions ansible/roles/ironic-inspector-rules/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,7 @@ compatible with the `auth` argument of `os_*` Ansible modules.

`ironic_inspector_cacert` is an optional path to a CA certificate.

`ironic_inspector_url` is the URL of Ironic Inspector API endpoint,
required if no authentication is used.
`ironic_inspector_cloud` is the name of a cloud in ``clouds.yaml``.

`ironic_inspector_rules` is a list of introspection rules which should
exist. See the Inspector rules API for details of parameters available
Expand Down
6 changes: 3 additions & 3 deletions ansible/roles/ironic-inspector-rules/defaults/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@ ironic_inspector_auth: {}
# CA certificate path.
ironic_inspector_cacert:

# Name of cloud in clouds.yaml.
ironic_inspector_cloud:

# Interface (public, internal, admin).
ironic_inspector_interface:

# URL of Ironic Inspector API endpoint.
ironic_inspector_url:

# List of rules which should exist. See the Inspector rules API for details of
# parameters available for rules.
ironic_inspector_rules: []
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,6 @@

# Store a list of import errors to report to the user.
IMPORT_ERRORS = []
try:
import ironic_inspector_client
except Exception as e:
IMPORT_ERRORS.append(e)
try:
import openstack
except Exception as e:
Expand Down Expand Up @@ -78,29 +74,21 @@
"""


def _build_client(module, cloud):
"""Create and return an Ironic inspector client."""
# Ensure the requested API version is supported.
# API 1.14 is the latest API version available in Rocky.
api_version = (1, 14)
client = ironic_inspector_client.v1.ClientV1(
inspector_url=module.params['inspector_url'],
interface=module.params['interface'],
session=cloud.session, region_name=module.params['region_name'],
api_version=api_version)
return client
def _get_client(module, cloud):
"""Return an Ironic inspector client."""
return cloud.baremetal_introspection


def _ensure_rule_present(module, client):
"""Ensure that an inspector rule is present."""
if module.params['uuid']:
try:
rule = client.rules.get(module.params['uuid'])
except ironic_inspector_client.ClientError as e:
if e.response.status_code != 404:
response = client.get('/rules/{}'.format(module.params['uuid']))
if not response.ok:
if response.status_code != 404:
module.fail_json(msg="Failed retrieving Inspector rule %s: %s"
% (module.params['uuid'], repr(e)))
else:
rule = response.json()
# Check whether the rule differs from the request.
keys = ('conditions', 'actions', 'description')
for key in keys:
Expand All @@ -121,23 +109,30 @@ def _ensure_rule_present(module, client):
# Rule differs - delete it before recreating.
_ensure_rule_absent(module, client)

client.rules.create(module.params['conditions'], module.params['actions'],
module.params['uuid'], module.params['description'])
rule = {
"conditions": module.params['conditions'],
"actions": module.params['actions'],
"description": module.params['description'],
"uuid": module.params['uuid'],
}
response = client.post("/rules", json=rule)
if not response.ok:
module.fail_json(msg="Failed creating Inspector rule %s: %s"
% (module.params['uuid'], response.text))
return True


def _ensure_rule_absent(module, client):
"""Ensure that an inspector rule is absent."""
if not module.params['uuid']:
module.fail_json(msg="UUID is required to ensure rules are absent")
try:
client.rules.delete(module.params['uuid'])
except ironic_inspector_client.ClientError as e:
response = client.delete("/rules/{}".format(module.params['uuid']))
if not response.ok:
# If the rule does not exist, no problem and no change.
if e.response.status_code == 404:
if response.status_code == 404:
return False
module.fail_json(msg="Failed retrieving Inspector rule %s: %s"
% (module.params['uuid'], repr(e)))
% (module.params['uuid'], response.text))
return True


Expand All @@ -149,7 +144,6 @@ def main():
uuid=dict(required=False),
state=dict(required=False, default='present',
choices=['present', 'absent']),
inspector_url=dict(required=False),
)
module_kwargs = openstack_module_kwargs()
module = AnsibleModule(argument_spec, **module_kwargs)
Expand All @@ -159,20 +153,9 @@ def main():
module.fail_json(msg="Import errors: %s" %
", ".join([repr(e) for e in IMPORT_ERRORS]))

if (module.params['auth_type'] in [None, 'None'] and
module.params['inspector_url'] is None):
module.fail_json(msg="Authentication appears disabled, please "
"define an inspector_url parameter")

if (module.params['inspector_url'] and
module.params['auth_type'] in [None, 'None']):
module.params['auth'] = dict(
endpoint=module.params['inspector_url']
)

sdk, cloud = openstack_cloud_from_module(module)
try:
client = _build_client(module, cloud)
client = _get_client(module, cloud)
if module.params["state"] == "present":
changed = _ensure_rule_present(module, client)
else:
Expand Down
12 changes: 1 addition & 11 deletions ansible/roles/ironic-inspector-rules/tasks/main.yml
Original file line number Diff line number Diff line change
@@ -1,26 +1,16 @@
---
- name: Ensure required Python packages are installed
pip:
name: "{{ item.name }}"
version: "{{ item.version | default(omit) }}"
state: latest
virtualenv: "{{ ironic_inspector_venv }}"
extra_args: "{% if ironic_inspector_upper_constraints_file %}-c {{ ironic_inspector_upper_constraints_file }}{% endif %}"
with_items:
- name: python-ironic-inspector-client

- name: Ensure introspection rules exist
vars:
ansible_python_interpreter: "{{ ironic_inspector_venv }}/bin/python"
os_ironic_inspector_rule:
auth_type: "{{ ironic_inspector_auth_type }}"
auth: "{{ ironic_inspector_auth }}"
cacert: "{{ ironic_inspector_cacert | default(omit, true) }}"
cloud: "{{ ironic_inspector_cloud | default(omit, true) }}"
interface: "{{ ironic_inspector_interface | default(omit, true) }}"
conditions: "{{ item.conditions }}"
actions: "{{ item.actions }}"
description: "{{ item.description | default(omit) }}"
uuid: "{{ item.uuid | default(item.description | to_uuid) | default(omit) }}"
state: present
inspector_url: "{{ ironic_inspector_url }}"
with_items: "{{ ironic_inspector_rules }}"
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,6 @@ use_firewalld: "{{ kolla_bifrost_use_firewalld }}"
# Firewalld zone used by Bifrost.
firewalld_internal_zone: "{{ kolla_bifrost_firewalld_internal_zone }}"

# Disable authentication for the Ironic and Inspector APIs.
noauth_mode: true

# Enable discovery of nodes in Ironic Inspector.
enable_inspector_discovery: true

Expand Down
45 changes: 45 additions & 0 deletions ansible/seed-credentials.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
---
# Copy the Bifrost clouds.yaml file and CA certificate (if one is in use) to
# the host. This allows us to access the Ironic and Inspector APIs outside of
# the Bifrost container.
- name: Ensure credentials are available on the host
hosts: seed
tags:
- seed-credentials
vars:
openstack_config_dir: "{{ ansible_facts.env.HOME }}/.config/openstack"
tasks:
- name: Ensure OpenStack config directory exists
file:
path: "{{ openstack_config_dir }}"
state: directory
mode: 0700

- name: Get clouds.yaml from Bifrost container
command:
cmd: docker exec bifrost_deploy cat /root/.config/openstack/clouds.yaml
changed_when: false
register: clouds_yaml
no_log: true

- name: Write clouds.yaml
copy:
content: |
{%- set clouds = clouds_yaml.stdout | from_yaml -%}
{%- for cloud in clouds.clouds.keys() | list -%}
{%- if 'cacert' in clouds.clouds[cloud] -%}
{%- set _ = clouds.clouds[cloud].update({'cacert': openstack_config_dir ~ '/bifrost.crt'}) -%}
{%- endif -%}
{%- endfor -%}
{{ clouds | to_nice_yaml }}
dest: "{{ openstack_config_dir }}/clouds.yaml"
mode: 0600

- name: Copy CA certificate from Bifrost container
vars:
clouds: "{{ clouds_yaml.stdout | from_yaml }}"
cacerts: "{{ clouds.clouds.values() | selectattr('cacert', 'defined') | map(attribute='cacert') | list }}"
command:
cmd: docker cp bifrost_deploy:{{ cacerts[0] }} {{ openstack_config_dir }}/bifrost.crt
changed_when: false
when: cacerts | length > 0
5 changes: 1 addition & 4 deletions ansible/seed-introspection-rules.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,7 @@
os_openstacksdk_state: latest
ironic_inspector_venv: "{{ virtualenv_path }}/openstacksdk"
ironic_inspector_upper_constraints_file: "{{ pip_upper_constraints_file }}"
# No auth required for Bifrost.
ironic_inspector_auth_type: None
ironic_inspector_auth: {}
ironic_inspector_url: "http://localhost:5050"
ironic_inspector_cloud: bifrost
ironic_inspector_rules: "{{ kolla_bifrost_inspector_rules }}"
# These variables may be referenced in the introspection rules.
inspector_rule_var_ipmi_username: "{{ kolla_bifrost_inspector_ipmi_username }}"
Expand Down
2 changes: 2 additions & 0 deletions kayobe/cli/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -714,6 +714,7 @@ def take_action(self, parsed_args):

self.run_kolla_ansible_seed(parsed_args, "deploy-bifrost")
playbooks = _build_playbook_list(
"seed-credentials",
"seed-introspection-rules",
"dell-switch-bmp")
self.run_kayobe_playbooks(parsed_args, playbooks)
Expand Down Expand Up @@ -748,6 +749,7 @@ def take_action(self, parsed_args):
self.run_kayobe_playbooks(parsed_args, playbooks)
self.run_kolla_ansible_seed(parsed_args, "upgrade-bifrost")
playbooks = _build_playbook_list(
"seed-credentials",
"seed-introspection-rules",
"dell-switch-bmp")
self.run_kayobe_playbooks(parsed_args, playbooks)
Expand Down
4 changes: 4 additions & 0 deletions kayobe/tests/unit/cli/test_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -881,6 +881,8 @@ def test_seed_service_deploy(self, mock_kolla_run, mock_run):
mock.call(
mock.ANY,
[
utils.get_data_files_path(
"ansible", "seed-credentials.yml"),
utils.get_data_files_path(
"ansible", "seed-introspection-rules.yml"),
utils.get_data_files_path(
Expand Down Expand Up @@ -936,6 +938,8 @@ def test_seed_service_upgrade(self, mock_kolla_run, mock_run):
mock.call(
mock.ANY,
[
utils.get_data_files_path(
"ansible", "seed-credentials.yml"),
utils.get_data_files_path(
"ansible",
"seed-introspection-rules.yml"),
Expand Down
9 changes: 9 additions & 0 deletions releasenotes/notes/seed-clouds-yaml-cbaf1961c5f8ceb0.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
features:
- |
Adds support for copying the Bifrost ``clouds.yaml`` file and optionally a
TLS CA certificate from the Bifrost container to the seed host. This makes
it possible to enable authentication and TLS for Bifrost services.
upgrade:
- |
Enables authentication by default in Bifrost.

0 comments on commit 0ff1f8c

Please sign in to comment.