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.

Sign up for GitHub

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

Jump to bottom

feat: Add support to configure parameters form insights-client.conf #177

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from
Draft

feat: Add support to configure parameters form insights-client.conf #177

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
94a83b6
Manage parameters in insights-client.conf
Tronde May 2, 2024
98455e2
Add documentation and fixed typo
Tronde May 2, 2024
b34b8ac
Fixed trailing space
Tronde May 3, 2024
aa4ab6d
Move insighs-client auth vars to another dict
Tronde May 10, 2024
7eb37a5
Removing redaction file parameters
Tronde May 13, 2024
efcc72e
Remove configuration option for the tags file
Tronde May 13, 2024
e2395bc
Implement file and file content redaction
Tronde May 21, 2024
ff0199b
Fixed several typos
Tronde May 30, 2024
1600b96
Use insights-client defaults for baseurl
Tronde May 31, 2024
808dd18
Fixed error in task conditions
Tronde May 31, 2024
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
77 changes: 77 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,25 @@ the system.
Whether the system is connected to Insights; valid values are `present`
(the default, to ensure connection), and `absent`.

```yaml
rhc_insights_auth:
authmethod: BASIC
username: null
password: null
```

Configures the authentication method; valid options for `authmethod` are `BASIC`
(the default), and `CERT`. The variables `username` and `password` configure
username and password when authmethod is `BASIC`.

```yaml
rhc_insights:
autoconfig: true
```

Whether the system attempts to auto configure with Satellite server, values
are `true` (the default), and `false`.

```yaml
rhc_insights:
autoupdate: true
Expand All @@ -173,6 +192,14 @@ Possible values of this variable:
* `{state: absent}`: the ansible host name is unset in the insights-client config file and Host Based Inventory (HBI) is updated to use the system host name.
* any other string value: the ansible host name is changed in Host Based Inventory (HBI).

```yaml
rhc_insights:
baseurl: null
```

Configures the Base URL for the Insights API.
If `baseurl: null` is set, the default of the `insights-client` will be used.

```yaml
rhc_insights:
display_name: "Example Host"
Expand All @@ -188,6 +215,56 @@ Possible values of this variable:

Note: If not set explicitly on registration, the display name is set to the hostname by default. It is not possible to automatically revert it to the hostname, but it can be set so manually.

```yaml
rhc_insights:
file_redaction:
commands: []
files: []
components: []
```

Specify lists of commands, files, and components to omit from output

```yaml
rhc_insights:
file_redaction:
commands: []
files: []
components: []
file_content_redaction:
keywords: []
patterns: []
regex_patterns: []
```

These are optional.
Specify lists of commands, files, components, keywords and patterns to omit from output.
NOTE: You cannot mix plain string matching and regular expression matching.
For more information on this topic read: [YAML-style denylist configuration for Red Hat Insights Client](https://access.redhat.com/articles/4511681).

```yaml
rhc_insights:
loglevel: DEFAULT
```

Configures the log level; valid options are DEBUG (the default), INFO, WARNING, ERROR, CRITICAL.

```yaml
rhc_insights:
obfuscate: false
```

Configures IP address obfuscation; valid values are `false` (the default),
and `true`.

```yaml
rhc_insights:
obfuscate_hostname: false
```

Configures hostname obfuscation; valid values are `false` (the default),
and `true`. Requires `obfuscate: true`.

```yaml
rhc_insights:
remediation: present
Expand Down
17 changes: 17 additions & 0 deletions defaults/main.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,28 @@ rhc_baseurl: null
rhc_environments: []
rhc_insights:
ansible_host: null
autoconfig: true
autoupdate: true
baseurl: null
display_name: null
file_redaction:
commands: []
files: []
components: []
file_content_redaction:
keywords: []
patterns: []
regex_patterns: []
loglevel: DEFAULT
obfuscate: false
obfuscate_hostname: false
remediation: present
state: present
tags: {}
rhc_insights_auth:
authmethod: BASIC
password: null
username: null
rhc_organization: null
rhc_proxy: {}
rhc_release: null
Expand Down
130 changes: 130 additions & 0 deletions tasks/insights-client.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,96 @@
insertafter: "#auto_update"
line: auto_update={{ rhc_insights.autoupdate | d(true) | bool }}

- name: Configure authmethod
when:
- rhc_insights_auth.authmethod is defined
- not rhc_insights_auth.authmethod is none
- rhc_insights_auth.authmethod != ""
- rhc_insights_auth.authmethod != __rhc_state_absent
- rhc_insights_auth.authmethod != omit
lineinfile:
path: "{{ __rhc_insights_conf }}"
regexp: "^authmethod="
state: present
line: authmethod={{ rhc_insights_auth.authmethod }}
check_mode: true
no_log: true

- name: Configure username for authmethod BASIC
when:
- rhc_insights_auth.authmethod = "BASIC"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- rhc_insights_auth.authmethod = "BASIC"
- rhc_insights_auth.authmethod is defined
- rhc_insights_auth.authmethod = "BASIC"

- rhc_insights_auth.authmethod != __rhc_state_absent
- rhc_insights_auth.username is defined
- not rhc_insights_auth.username is none
- rhc_insights_auth.username != ""
- rhc_insights_auth.username != omit
lineinfile:
path: "{{ __rhc_insights_conf }}"
regexp: "^username="
state: present
line: username={{ rhc_insights_auth.username }}
check_mode: true
no_log: true

- name: Configure password for authmethod BASIC
when:
- rhc_insights_auth.authmethod == "BASIC"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- rhc_insights_auth.authmethod == "BASIC"
- rhc_insights_auth.authmethod is defined
- rhc_insights_auth.authmethod == "BASIC"

- rhc_insights_auth.authmethod != __rhc_state_absent
- rhc_insights_auth.password is defined
- not rhc_insights_auth.password is none
- rhc_insights_auth.password != ""
- rhc_insights_auth.password != omit
lineinfile:
path: "{{ __rhc_insights_conf }}"
regexp: "^password="
state: present
line: password={{ rhc_insights_auth.password }}
check_mode: true
Tronde marked this conversation as resolved.
Show resolved Hide resolved
no_log: true

- name: Configure Base URL for the Insights API
when:
- rhc_insights.baseurl is defined
- not rhc_insights.baseurl is none
- rhc_insights.baseurl != ""
- rhc_insights.baseurl != __rhc_state_absent
- rhc_insights.baseurl != omit
lineinfile:
path: "{{ __rhc_insights_conf }}"
regexp: "^base_url="
state: present
line: base_url={{ rhc_insights.baseurl }}
check_mode: true

- name: Configure IP address obfuscation
when:
- rhc_insights.obfuscate is defined
- not rhc_insights.obfuscate is none
- rhc_insights.obfuscate != ""
- rhc_insights.obfuscate != __rhc_state_absent
- rhc_insights.obfuscate != omit
lineinfile:
path: "{{ __rhc_insights_conf }}"
regexp: "^obfuscate="
state: present
line: obfuscate={{ rhc_insights.obfuscate }}
check_mode: true

- name: Configure hostname obfuscation
when:
- rhc_insights.obfuscate == "true"
- rhc_insights.hostname_obfuscate is defined
- not rhc_insights.hostname_obfuscate is none
- rhc_insights.hostname_obfuscate != ""
- rhc_insights.hostname_obfuscate != __rhc_state_absent
- rhc_insights.hostname_obfuscate != omit
lineinfile:
path: "{{ __rhc_insights_conf }}"
regexp: "^hostname_obfuscate="
state: present
line: hostname_obfuscate={{ rhc_insights.hostname_obfuscate }}
check_mode: true

- name: Check ansible host in insights-client config
when:
- rhc_insights.ansible_host is defined
Expand Down Expand Up @@ -133,6 +223,46 @@
or "Registered" in __rhc_insights_status.stdout
changed_when: true

- name: Configure file redaction
when: (rhc_insights.file_redaction.commands is defined) or
Copy link
Contributor

Choose a reason for hiding this comment

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

Does there need to be a check first for rhc_insights.file_redaction is defined?

Copy link
Author

Choose a reason for hiding this comment

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

AFAICS rhc_insights.file_redaction is defined in defaults/main.yml. As defaults/main.yml as well as tasks/insights-client.yml are part of one role and shipped together I do not see a scenario where rhc_insights.file_redaction would not be defined. So with my current knowledge and understanding I would say there is no need to check first for rhc_insights.file_redaction is defined.

I see that most if not all tasks in tasks/insights-client.yml check whether variables are defined although all of these variables are defined in defaults/main.yml. TBH I do not understand why these checks are necessary. Are they in place for "just in case" scenarios or is there some reason I do not see?

Following my own argument I would suggest removing all the var is defined checks where the var is defined in defaults/main.yml as the check is not striclty necessary. But I might miss a good reason here.

Copy link
Contributor

Choose a reason for hiding this comment

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

AFAICS rhc_insights.file_redaction is defined in defaults/main.yml.

That unfortunately doesn't mean anything.

As defaults/main.yml as well as tasks/insights-client.yml are part of one role and shipped together I do not see a scenario where rhc_insights.file_redaction would not be defined.

What if I run the role like this?

inventory:....
  rhc_insights:
    foo: bar

Defining a host variable takes precedence over settings in defaults/main.yml. The same applies to setting the variable at the play level or as one of the vars in the include_role call. Setting rhc_insights in this way completely replaces the existing contents defined in defaults/main.yml. https://docs.ansible.com/ansible/latest/reference_appendices/general_precedence.html#variables

So - while "top level" variables like rhc_insights will always be defined, their "sub-parameters" may or may not be defined - that is - rhc_insights is defined will always return true, but rhc_insights.file_redaction is defined can return true or false.

If you don't believe me, try running the role with different values and see what happens.

So with my current knowledge and understanding I would say there is no need to check first for rhc_insights.file_redaction is defined.

I see that most if not all tasks in tasks/insights-client.yml check whether variables are defined although all of these variables are defined in defaults/main.yml. TBH I do not understand why these checks are necessary. Are they in place for "just in case" scenarios or is there some reason I do not see?

Yes, and if some checks are missing, they should be added in another PR.

TBH, I'm really not sure why "sub-parameter" keys and values are defined in defaults/main.yml, except possibly as a convenience when the user does not provide a value for rhc_insights at all. There is no mechanism in Ansible like "merge the default dict valued parameter rhc_insights defined in defaults/main.yml with the rhc_insights parameter given by the user". If the user defines rhc_insights, then the value in defaults/main.yml is completely replaced.

Following my own argument I would suggest removing all the var is defined checks where the var is defined in defaults/main.yml as the check is not striclty necessary. But I might miss a good reason here.

Copy link
Author

Choose a reason for hiding this comment

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

What if I run the role like this?

inventory:....
  rhc_insights:
    foo: bar

…

If you don't believe me, try running the role with different values and see what happens.

Thank you for reminding me about variable precedence and giving me a good example when things will break. I didn't find this as my own tests are still flawed and incomplete.

I cannot thank you enough for bearing with me and share your thoughts and insights on this as it is a great learning opportunity for myself.

TBH, I'm really not sure why "sub-parameter" keys and values are defined in defaults/main.yml, except possibly as a convenience when the user does not provide a value for rhc_insights at all. There is no mechanism in Ansible like "merge the default dict valued parameter rhc_insights defined in defaults/main.yml with the rhc_insights parameter given by the user". If the user defines rhc_insights, then the value in defaults/main.yml is completely replaced.

Well, putting all variables, parameters and arguments accepted by the role into defaults/main.yml gives users a single place to look what input parameters are accepted. That's in alignment with this section of the automation good practices. And I understood that every condition needs to check whether a variable is defined or not as the whole dict defined defaults/main.yml is replaced when users specify this elsewhere according to variable precedence.

I'm going to check the conditionals in the tasks I've added so far. Please allow me some time to go through them.

(rhc_insights.file_redaction.files is defined) or
(rhc_insights.file_redaction.components is defined)
block:
- name: Create file-redaction.yaml from template
template:
src: templates/file-redaction.yaml.j2
dest: /etc/insights-client/file-redaction.yaml
owner: root
group: root
mode: 0660
- name: Configure path to file-redaction.yaml in insights-client.conf
lineinfile:
path: "{{ __rhc_insights_conf }}"
regexp: "^redaction_file="
insertafter: "#redaction_file="
line: redaction_file=/etc/insights-client/file-redaction.yaml

- name: Configure file content redaction
when: (rhc_insights.file_content_redaction.keywords is defined) or
Copy link
Contributor

Choose a reason for hiding this comment

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

Does there need to be a check first for rhc_insights.file_content_redaction is defined?

(rhc_insights.file_content_redaction.patterns is defined) or
(rhc_insights.file_content_redaction.regex_patterns is defined)
block:
- name: Create file-content-redaction.yaml from template
template:
src: templates/file-content-redaction.yaml.j2
dest: /etc/insights-client/file-content-redaction.yaml
owner: root
group: root
mode: 0660
- name: Configure path to file-redaction.yaml in insights-client.conf
lineinfile:
path: "{{ __rhc_insights_conf }}"
regexp: "^content_redaction_file="
insertafter: "#content_redaction_file="
line: >-
"content_redaction_file=/etc/insights-client/
file-content-redaction.yaml"

- name: Register insights-client
shell: insights-client --register & wait
when:
Expand Down
43 changes: 43 additions & 0 deletions templates/file-content-redaction.yaml.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
# file-conrent-redaction.yaml
# Docs: https://access.redhat.com/articles/4511681
---
# Omit lines from files and command output using parameters listed here.
# Lines matching the parameters specified will be omitted
# in the order that the parameters are given, e.g.,
#
# patterns:
# - "example_string_1"
# - "example_string_2"
#
# Lines containing "example_string_1" or "example_string_2" will be
# omitted from output.
#
# To use regular expressions, wrap the array with "regex" like the following example:
#
# patterns:
# regex:
# - abc.*
# - localhost[[:digit:]]
#
# Lines matching these regular expressions will be omitted
# from output.
# NOTE: You cannot mix plain string matching and regular expression matching.
{% if rhc_insights.file_content_redaction.keywords %}
keywords:
{% for keyword in rhc_insights.file_content_redaction.keywords %}
- {{ keyword }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible that this or any of the below values will need to be quoted? will it be possible that the values will contain yaml metacharacters?

Copy link
Author

Choose a reason for hiding this comment

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

For patterns (with regex) that would be possible as the examples in YAML-style denylist configuration for Red Hat Insights Client show characters that need to be escaped.

For keywords, I tend to escape them just to be sure. Do you agree?

Copy link
Contributor

Choose a reason for hiding this comment

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

This is tricky - afaik there is no "regex quote" or "regex escape" filter in Jinja or Ansible (like there is for shell metacharacters and the quote filter). If you use - "{{ pattern }}" I think this will cause problems if pattern contains " - same with '{{ pattern }}' and ' - however, this might be safe:

  - >-
   {{ pattern }}

{% endfor %}
{% endif %}
{% if rhc_insights.file_content_redaction.patterns %}
patterns:
{% for pattern in rhc_insights.file_content_redaction.patterns %}
- {{ pattern }}
{% endfor %}
{% endif %}
{% if rhc_insights.file_content_redaction.regex_patterns %}
patterns:
regex:
{% for regex in rhc_insights.file_content_redaction.regex_patterns %}
- {{ regex }}
{% endfor %}
{% endif %}
37 changes: 37 additions & 0 deletions templates/file-redaction.yaml.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
# file-redaction.yaml
# Docs: https://access.redhat.com/articles/4511681
---
# Omit entire output of commands
# Commands can be specified either by full command or
# by the "symbolic_name" listed in /etc/insights-client/.cache.json
{% if rhc_insights.file_redaction.commands %}
commands:
{% for command in rhc_insights.file_redaction.commands %}
- {{ command }}
{% endfor %}
{% endif %}

# Omit entire output of files
# Files can be specified either by full filename or
# by the "symbolic_name" listed in .cache.json
{% if rhc_insights.file_redaction.files %}
files:
{% for file in rhc_insights.file_redaction.files %}
- {{ file }}
{% endfor %}
{% endif %}

# Omit insights-core components
# Refer to the Datasource Catalog here for a full list of available insights-core components,
# and the commands/files they correspond to.
# See items listed under "General Datasources":
# https://insights-core.readthedocs.io/en/latest/specs_catalog.html
#
# Components specified here must be listed with the fully qualified name, i.e.
# must be prefixed with "insights.specs.default.DefaultSpecs."
{% if rhc_insights.file_redaction.components %}
components:
{% for component in rhc_insights.file_redaction.components %}
- {{ component }}
{% endfor %}
{% endif %}