cyberark / conjur-inspect

Apache License 2.0
3 stars 0 forks source link

Add integration tests and other quality improvements #20

Closed micahlee closed 1 year ago

micahlee commented 1 year ago

Desired Outcome

The desired outcome of this PR is to separate the quality-of-life improvements and clean-ups from #19 into a separate PR for easier reviewing.

Implemented Changes

This PR is devise of many discrete commits that perform small clean-ups or add quality-of-life improvements (e.g. dev environment, integration tests). It should be review by commit.

Connected Issue/Story

This is related to #19.

Definition of Done

At least 1 todo must be completed in the sections below for the PR to be merged.

Changelog

Test coverage

Documentation

Behavior

Security

micahlee commented 1 year ago

Hey @adamouamani,

  • Hanit has recently asked us to support rhel9 as well - would it be easy to add this later?

I don't know of any reason the preflight tool wouldn't run on RHEL 9 now. If you're referring to the use of rhel/ubi8 for the test containers, we could easily bump that to 9 if that would be helpful. The goal of those containers right now is essentially to have "executable documentation" to show how you would gather the dependencies and run them on RHEL vs Debian based environment, which are the two I've seen in customer environments before.

That said, if you take a look a the release page, you can see we're building the binary for a variety of platforms, so the portability of pre-flight should be much greater than just RHEL or Ubuntu. For example, you can run the tool itself directly on MacOS. Some of the checks may not be able to run successfully depending on the platform (e.g. the IOPs check with fio will only support linux platforms because it depends on the linux-only libaio library).

  • Are you planning to constrain the containers memory / cpu as part of the pre-flight tool

I'm not sure what you mean by this, can you elaborate?

  • What about running rootless - are we going to check for rootless requirements such as slirp4netns etc?

There's a WIP PR for adding container runtime checks here: https://github.com/conjurinc/conjur-preflight/pull/15. We'll be able to pull out any of the info from podman info that's helpful. The issue for that is ONYX-31571.

My intention is also to have the tool save all of the raw data as well (so we would save the entire podman info result, even if the high-level report only extracts key values). The issue that captures that in the roadmap is ONYX-29742.

Regarding rootless, in the current model, the preflight tool interacts with podman with whatever privilege level the tool itself has. So for rootless podman checks, you would run the tool without root. To check with root permissions, you would run it as sudo conjur-preflight.

adamouamani commented 1 year ago
  • Are you planning to constrain the containers memory / cpu as part of the pre-flight tool

I'm not sure what you mean by this, can you elaborate?

Hey Micah - My question above was not very clear. I was looking at this pre-flight tool as a way to solve two different issues.

For the later (ii), I was wondering if we want to check what limits (memory/cpu) a customer may be running with a container and if it does not meet minimum requirements (beyond total available memory and cpu). for example using docker stats to check --memory or --cpus-limit? I'll also take a look at your PR below for container runtime checks

micahlee commented 1 year ago
  • Are you planning to constrain the containers memory / cpu as part of the pre-flight tool

I'm not sure what you mean by this, can you elaborate?

Hey Micah - My question above was not very clear. I was looking at this pre-flight tool as a way to solve two different issues.

    1. As a pre-flight before deploying or POC
    1. As a support tool after a customer calls cyberark

For the later (ii), I was wondering if we want to check what limits (memory/cpu) a customer may be running with a container and if it does not meet minimum requirements (beyond total available memory and cpu). for example using docker stats to check --memory or --cpus-limit? I'll also take a look at your PR below for container runtime checks

Hey @adamouamani, ah I see what you mean. Yes, getting the docker inspect info for the Conjur container is a common data point we collect in support cases.

That PR won't include that yet. It's more focused on the container runtime system settings than a particular container. I've added this to the comment here capturing additional ideas on the epic to add as stories as time permits. Thanks!