Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for host certificate deletion, display and installation #483

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions plugins/module_utils/network/sonic/argspec/pki/pki.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,16 @@ def __init__(self, **kwargs):
'name': {'required': True, 'type': 'str'}
},
'type': 'list'
},
'host_cert': {
'elements': 'dict',
'options': {
'file_path': {'type': 'str'},
'file_name': {'type': 'str'},
'fips_cert': {'type': 'bool', 'default': False},
'key_path': {'type': 'str'}
},
'type': 'list'
}
},
'type': 'dict'
Expand Down
33 changes: 33 additions & 0 deletions plugins/module_utils/network/sonic/config/pki/pki.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,13 @@
SECURITY_PROFILE_PATH = (
"data/openconfig-pki:pki/security-profiles/security-profile"
)
INSTALL_PATH = "operations/openconfig-pki-rpc:crypto-host-cert-install"
DELETE_PATH = "operations/openconfig-pki-rpc:crypto-host-cert-delete"

PATCH = "patch"
DELETE = "delete"
PUT = "put"
POST = 'post'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
POST = 'post'
POST = "post"

A little nitpick but for consistency you should use double quotes here

TEST_KEYS = [
{"security_profiles": {"profile_name": ""}},
{"trust_stores": {"name": ""}},
Expand Down Expand Up @@ -322,6 +325,20 @@ def _state_merged(self, want, have, diff):
}
)

# Handle INSTALL for certificates
for cert in commands.get("host_cert") or []:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain why the empty list is necessary here? I don't immediately see a function of that or clause here.

if isinstance(cert, dict) and cert.get("file_path") and cert.get("key_path"):
requests.append(
{
"path": INSTALL_PATH,
"method": POST,
"data": {"openconfig-pki-rpc:input":
{"file-path": cert.get('file_path'),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
{"file-path": cert.get('file_path'),
{"file-path": cert.get("file_path"),

A little nitpick but for consistency you should use double quotes here

"key-path": cert.get('key_path'),
"fips-cert": cert.get('fips_cert')}}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"fips-cert": cert.get('fips_cert')}}
"fips-cert": cert.get("fips_cert")}}

A little nitpick but for consistency you should use double quotes here

}
)

if commands and requests:
commands = update_states(commands, "merged")
else:
Expand Down Expand Up @@ -371,6 +388,22 @@ def _state_deleted(self, want, have, diff):
if ts.get("name") in current_ts:
requests.extend(mk_ts_delete(ts, have))

# Handle DELETE for certificates
for cert in commands.get("host_cert") or []:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain why the empty list is necessary here? I don't immediately see a function of that or clause here.

if isinstance(cert, dict) and cert.get("file_name"):
requests.append(
{
"path": DELETE_PATH,
"method": POST,
"data": {
"openconfig-pki-rpc:input": {
"file-name": cert.get("file_name"),
"fips-cert": cert.get("fips_cert", False),
}
},
}
)

if commands and requests:
commands = update_states([commands], "deleted")
else:
Expand Down
33 changes: 32 additions & 1 deletion plugins/module_utils/network/sonic/facts/pki/pki.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@

pki_path = "data/openconfig-pki:pki/"
security_profiles_path = "data/openconfig-pki:pki/security-profiles"

cert_get_path = "operations/openconfig-pki-rpc:crypto-host-cert-display"

class PkiFacts(object):
"""The sonic pki fact class"""
Expand Down Expand Up @@ -108,6 +108,21 @@ def populate_facts(self, connection, ansible_facts, data=None):

objs["trust_stores"] = rep_conf

cert_data = self.get_cert()
cert_list = []
if cert_data and len(cert_data) > 0 and "openconfig-pki-rpc:output" in cert_data[0][1]:
cert_data = cert_data[0][1].get("openconfig-pki-rpc:output", {})
if cert_data and "filename" in cert_data:
filenames = cert_data.get("filename", []) or []
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the "or []" do anything in this statement? If the get() does not find anything with the key "filename" it will return an empty list. I am unclear on what the "or []" snippet does here.

fips_certs = cert_data.get("fips-cert", []) or []
if len(filenames) > 0 and len(filenames) == len(fips_certs):
for file_name, fips_cert in zip(filenames, fips_certs):
cert_list.append({
"file_name": file_name,
"fips_cert": fips_cert,
})
objs["host_cert"] = cert_list

ansible_facts["ansible_network_resources"].pop("pki", None)
facts = {}
if objs:
Expand All @@ -131,6 +146,22 @@ def get_pki(self):

return response

def get_cert(self):
request = {
"path": cert_get_path,
"method": "post",
"data": {"openconfig-pki-rpc:input":
{"file-name": "all"}}
}
try:
response = edit_config(
self._module, to_request(self._module, request)
)
except ConnectionError as exc:
self._module.fail_json(msg=str(exc), code=exc.code)

return response

def render_config(self, spec, conf):
"""
Render config as dictionary structure and delete keys
Expand Down
24 changes: 23 additions & 1 deletion tests/regression/roles/sonic_pki/defaults/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,29 @@ tests:
- name: newts
ca_name: CA

test_delete_all:
- name: test_case_09
description: Merge parameter of host cert install
state: merged
input:
host_cert:
- file_path: "home://cert1.crt"
key_path: "home://cert1.key"
fips_cert: true
- file_path: "home://cert2.crt"
key_path: "home://cert2.key"
fips_cert: false

- name: test_case_10
description: Delete parameter of host cert delete
state: deleted
input:
host_cert:
- file_name: "cert1"
fips_cert: true
- file_name: "cert2"
fips_cert: false

test_delete_all:
- name: test_case_11
description: Delete all PKI configurations
state: deleted