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

[juju] Add plugin option for Juju state reporting #3803

Closed
wants to merge 9 commits into from

Conversation

MichaelThamm
Copy link
Contributor

@MichaelThamm MichaelThamm commented Oct 10, 2024

This change modifies the Juju plugin to optionally capture Juju state (controllers, models, applications, units) information. The capture assumes that juju is installed on the machine where sos is called, and that the juju user has superuser privilege to the current (or requested) controllers.

This option is disabled by default because, while the information can be very useful, the collection acts on the live Juju state.

Use the plugin with the feature:

  • sos report --only-plugin juju -k juju.juju-state=True

Run with a specific user (who has access to the controllers specified). This defaults to ubuntu:

  • -k juju.juju-user=super_admin

If necessary, you can filter by controllers or models with cluster options.

  • -k juju.controllers="controller_a controller_b"
  • -k juju.models="controller_a:model_x controller_b:model_y"

If nothing is supplied for either the controllers or models options, the report will include all state information for them respectively.


Please place an 'X' inside each '[]' to confirm you adhere to our Contributor Guidelines

  • Is the commit message split over multiple lines and hard-wrapped at 72 characters?
  • Is the subject and message clear and concise?
  • Does the subject start with [plugin_name] if submitting a plugin patch or a [section_name] if part of the core sosreport code?
  • Does the commit contain a Signed-off-by: First Lastname [email protected]?
  • Are any related Issues or existing PRs properly referenced via a Closes (Issue) or Resolved (PR) line?
  • Are all passwords or private data gathered by this PR obfuscated?
  • Add tests to ensure results are scrubbed / functioning

Copy link

Congratulations! One of the builds has completed. 🍾

You can install the built RPMs by following these steps:

  • sudo yum install -y dnf-plugins-core on RHEL 8
  • sudo dnf install -y dnf-plugins-core on Fedora
  • dnf copr enable packit/sosreport-sos-3803
  • And now you can install the packages.

Please note that the RPMs should be used only in a testing environment.

Copy link
Member

@arif-ali arif-ali left a comment

Choose a reason for hiding this comment

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

As well as turboturtle comments, can we also look at the flake/pylint issues as well is any updated PR

sos/report/plugins/juju.py Outdated Show resolved Hide resolved
sos/report/plugins/juju.py Outdated Show resolved Hide resolved
sos/report/plugins/juju.py Show resolved Hide resolved
@jcastill
Copy link
Member

A couple of very small comments from me.
The DCO is still failing, I think you are missing '-s' while running 'git commit'.
Last thing, please make sure you squash the two commits you have at the moment into one.

@jcastill
Copy link
Member

@MichaelThamm remember to squash all the commits into one and sign it

@MichaelThamm MichaelThamm changed the title Feat: Report on juju environment [Juju] Add plugin option for Juju state reporting Oct 14, 2024
@MichaelThamm MichaelThamm changed the title [Juju] Add plugin option for Juju state reporting [juju] Add plugin option for Juju state reporting Oct 14, 2024
@MichaelThamm
Copy link
Contributor Author

@arif-ali or @jcastill I imagine that I need to obfuscate some of the following information which is included in the juju controllers command output:

        "lxd": {
            "user": "admin",
            "recent-server": IP:Port,
            "uuid": "ac03821c-361e-4daf-83c9-a712cd56b8d4",
            "api-endpoints": [IP:Port],
            "ca-cert": "-----BEGIN CERTIFICATE-----
            ...
            -----END CERTIFICATE-----",
        },

Is there a need to obfuscate these things or is it just passwords, tokens, keys as mentioned in obfuscate?

@arif-ali
Copy link
Member

@MichaelThamm The IP addresses and endpoints, not so, as they would be typically handled by --clean or sos clean, if indeed these needed to be obfuscated.

In terms of the certs, you can get the example of the sunbeam.py plugin, where we just needs self.do_cmd_private_sub('juju controllers') in the postproc() function. This will automatically obfuscate any command that showed the cert.

sos/report/plugins/juju.py Outdated Show resolved Hide resolved
sos/report/plugins/juju.py Outdated Show resolved Hide resolved
sos/report/plugins/juju.py Show resolved Hide resolved
sos/report/plugins/juju.py Outdated Show resolved Hide resolved
sos/report/plugins/juju.py Outdated Show resolved Hide resolved
@MichaelThamm
Copy link
Contributor Author

@arif-ali

In terms of the certs, you can get the example of the sunbeam.py plugin, where we just needs self.do_cmd_private_sub('juju controllers') in the postproc() function. This will automatically obfuscate any command that showed the cert.

When I run: self.do_cmd_private_sub('juju controllers --format=json') I see this in the logs:

2024-10-15 10:13:54,360 DEBUG: [plugin:juju] substituting '-----SCRUBBED' for '----(?:-| )BEGIN.*?----(?:-| )END' in commands matching '*juju controllers --format=json*'

However, I do not see results in the commands dir of the sos report tar.xz file

This method does not have a filename parameter either. Not sure how to continue with this?

@arif-ali
Copy link
Member

When I run: self.do_cmd_private_sub('juju controllers --format=json') I see this in the logs:

2024-10-15 10:13:54,360 DEBUG: [plugin:juju] substituting '-----SCRUBBED' for '----(?:-| )BEGIN.*?----(?:-| )END' in commands matching '*juju controllers --format=json*'

However, I do not see results in the commands dir of the sos report tar.xz file

This method does not have a filename parameter either. Not sure how to continue with this?

You can just specify juju controllers in the function, and it will do a match. In the sunbeam.py, I did the same with doing a self.collect_cmd_output("juju controllers --format json"), and then used the juju controllers in the self.do_cmd_private_sub()

@MichaelThamm
Copy link
Contributor Author

I think the PR is almost ready for review, but I noticed we have tests for the Juju plugin. Is there any need to update this since the test for scrubbing seems to be sufficient? I also tested the scrubbing for juju controllers manually and it passes.

@MichaelThamm MichaelThamm marked this pull request as ready for review October 15, 2024 15:41
sos/report/plugins/juju.py Show resolved Hide resolved

# Specific models
if self.get_option("models"):
for model in self.get_option("models").split(" "):
Copy link
Member

Choose a reason for hiding this comment

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

Ditto, we prefer plugin options to use : as a delimiter. We may need to add this to the plugin guide, I'll check this shortly.

sos/report/plugins/juju.py Outdated Show resolved Hide resolved
sos/report/plugins/juju.py Outdated Show resolved Hide resolved
sos/report/plugins/juju.py Show resolved Hide resolved
@jcastill
Copy link
Member

@MichaelThamm remember to squash the three commits into one, and to sign the commit (that is what's causing the DCO failure).

@arif-ali
Copy link
Member

arif-ali commented Nov 19, 2024

You can ignore the pylint errors, seems like new tests have come through for too-many-positional-arguments

EDIT, in-fact if you rebased to main you'll pick up the right pylintrc :) but I am cool either way

@arif-ali arif-ali added this to the 4.8.2 milestone Nov 19, 2024
jcastill and others added 5 commits November 19, 2024 10:38
Disable alert for too-many-positional-arguments

Signed-off-by: Jose Castillo <[email protected]>
Starting with containerd 1.6.27 and 1.7.12, warnings are issued for the
use of deprecated features.  The new `ctr deprecations list` command can
be used to retrieve warnings.

Closes: sosreport#3786
Signed-off-by: Samuel Karp <[email protected]>
HTTP_PROXY or similar env.variables can contain credentials we must
scrub. The variables or directly credentials of a http(s) URL can be
specified in several places the commit deals with.

Futher, update apt plugin to use the new do_paths_httpp_sub method.

Resolves: sosreport#3789

Signed-off-by: Pavel Moravec <[email protected]>
Resolves: sosreport#3788

Signed-off-by: Pablo Fernández Rodríguez<[email protected]>
Signed-off-by: Pablo Fernández Rodríguez <[email protected]>
These columns are either empty, containing passwords or some encoded
data.

Get the *remaining* column names and query for them.

If the query for column names fail, failover to current "SELECT *".

Relevant: sosreport#3783
Resolves: sosreport#3784

Signed-off-by: Pavel Moravec <[email protected]>
adam-vest and others added 3 commits November 19, 2024 10:38
Capture commands related to nvidia container toolkit.

Related: RHEL-58172

Signed-off-by: Jose Castillo <[email protected]>
Signed-off-by: Ponnuvel Palaniyappan <[email protected]>
@MichaelThamm
Copy link
Contributor Author

MichaelThamm commented Nov 19, 2024

Closing in favour of:

Due to the jumbled CI mess I created.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants