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 Debian 11 #7715

Merged
merged 4 commits into from
Oct 18, 2021
Merged

Conversation

mdedonno1337
Copy link
Contributor

@mdedonno1337 mdedonno1337 commented Oct 8, 2021

Description:

Rationale:

Content for Debian 11 does not exists

#### Notes:

- Please note that the check for the security repository in the /etc/apt/sources.list file is not done correctly at the moment. A commit shall be done to patch this before mergin in master (I'm fighting with some segmentation fault in the binary at the moment...)

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Used by openshift-ci bot. label Oct 8, 2021
@pep8speaks
Copy link

pep8speaks commented Oct 8, 2021

Hello @mdedonno1337! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2021-10-14 16:22:24 UTC

@openshift-ci openshift-ci bot added the needs-ok-to-test Used by openshift-ci bot. label Oct 8, 2021
@openshift-ci
Copy link

openshift-ci bot commented Oct 8, 2021

Hi @mdedonno1337. Thanks for your PR.

I'm waiting for a ComplianceAsCode member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@mdedonno1337
Copy link
Contributor Author

mdedonno1337 commented Oct 9, 2021

The issue #7718 is blocking this PR; with the patch proposed applied or the commit reverted, the check on a Debian 11 machine is done without errors.

@mdedonno1337 mdedonno1337 changed the title WIP: Add support for Debian 11 Add support for Debian 11 Oct 10, 2021
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Used by openshift-ci bot. label Oct 10, 2021
@ggbecker
Copy link
Member

What do you think about adding a github actions job here that would build the debian (9,10,11) content and run some tests... you can basically copy paste one of the validate-* and adapt to this distribution... at least we can validate it builds and passes tests.

https://github.com/ComplianceAsCode/content/blob/master/.github/workflows/gate.yaml#L8

@mdedonno1337
Copy link
Contributor Author

mdedonno1337 commented Oct 13, 2021

Never done actions on Github (only on Gitlab usually), but I will try, thanks for the idea!
I will have to search some tutoriaal, in particular on how to test the build on my fork first (I dont like to PR without testing...)

@ggbecker ggbecker self-assigned this Oct 13, 2021
@mdedonno1337
Copy link
Contributor Author

Probably only an issue with the scanner.
With the patch proposed in OpenSCAP/openscap#1815, the scann can be done in a Debian 11 machine without any problems.

@mdedonno1337
Copy link
Contributor Author

The compilation of this repo require the openscap binary to be present.
However, openscap is not present in Debian 11 (https://tracker.debian.org/pkg/openscap ; only available in the next release (unstable) and the old one (old-stable)).

We could git clone and build the binary from source...

What do you advice on that ?

@ggbecker
Copy link
Member

ggbecker commented Oct 14, 2021

The compilation of this repo require the openscap binary to be present. However, openscap is not present in Debian 11 (https://tracker.debian.org/pkg/openscap ; only available in the next release (unstable) and the old one (old-stable)).

We could git clone and build the binary from source...

What do you advice on that ?

You can rather use debian10 to build the content. That's not a problem. I imagine you would have to use container though, something like this:

  validate-debian:
    name: Build, Test on Debian 10 (Container)
    runs-on: ubuntu-latest
    container:
      image: debian:10 #<---- double check here
    steps:
      - name: Install Deps
        uses: mstksg/get-package@master
        with:
          apt-get: cmake ninja-build libopenscap8 libxml2-utils expat xsltproc python3-jinja2 python3-yaml ansible-lint python3-github bats python3-pytest python3-pytest-cov #<---- double check here
      - name: Install deps python
        run: pip install ruamel.yaml yamlpath
      - name: Checkout
        uses: actions/checkout@v2
      - name: Build
        run: |-
          ./build_product \
              debian9 \
              debian10 \
              debian11
        env:
          ADDITIONAL_CMAKE_OPTIONS: "-DSSG_ANSIBLE_PLAYBOOKS_PER_RULE_ENABLED=ON"
      - name: Test
        run: ctest -j2 --output-on-failure -E unique-stigids
        working-directory: ./build

@mdedonno1337
Copy link
Contributor Author

Ok, works for me.
The build done on Debian 10 works for debian 9, 10 and 11.

The PR is OK to test/merge on my side.

@ggbecker
Copy link
Member

The problem in tests:

Gating / Content Test Filtering on Ubuntu Latest (pull_request)
SSGTS / Build Content (pull_request)

is reported here: ComplianceAsCode/content-test-filtering#20

It can be addressed later.

@ggbecker ggbecker added this to the 0.1.59 milestone Oct 18, 2021
Copy link
Member

@ggbecker ggbecker left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for your contribution.

@ggbecker ggbecker merged commit 5bcb87d into ComplianceAsCode:master Oct 18, 2021
@mdedonno1337 mdedonno1337 deleted the debian11 branch October 18, 2021 11:54
@ggbecker
Copy link
Member

ggbecker commented Oct 18, 2021

@mdedonno1337 I believe we need to update the package selection in the build job to install YAML through pip to fix this problem(the package might be too old): https://github.com/ComplianceAsCode/content/pull/7758/checks?check_run_id=3927494067

  1. remove python3-yaml from apt-get
  2. add pip install pyyaml (we maybe have to use the --upgrade option as well)
    https://github.com/ComplianceAsCode/content/blob/master/.github/workflows/gate.yaml#L33

@mdedonno1337
Copy link
Contributor Author

mdedonno1337 commented Oct 18, 2021

I will have a look on that.

@ggbecker
Copy link
Member

I will have a look on that. Should I re-push here or is there a way to push on the other PR ?

I have just created #7760 it seems to work

@ggbecker
Copy link
Member

I will have a look on that. Should I re-push here or is there a way to push on the other PR ?

I have just created #7760 it seems to work

It fixed one problem, but then appear another ;(

https://github.com/ComplianceAsCode/content/pull/7760/checks?check_run_id=3928706959

@mdedonno1337
Copy link
Contributor Author

I will have a look on that. Should I re-push here or is there a way to push on the other PR ?

I have just created #7760 it seems to work

It fixed one problem, but then appear another ;(

https://github.com/ComplianceAsCode/content/pull/7760/checks?check_run_id=3928706959

I will have a look.

@ggbecker
Copy link
Member

ggbecker commented Oct 18, 2021

I will have a look on that. Should I re-push here or is there a way to push on the other PR ?

I have just created #7760 it seems to work

It fixed one problem, but then appear another ;(
https://github.com/ComplianceAsCode/content/pull/7760/checks?check_run_id=3928706959

I will have a look.

I'm installing the same version as the one in Fedora, let's see what happens. #7760

it was probably this: yaml/pyyaml#576 it installed the version 6 of pyyaml xD

@yuumasato yuumasato added the Highlight This PR/Issue should make it to the featured changelog. label Nov 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Highlight This PR/Issue should make it to the featured changelog. needs-ok-to-test Used by openshift-ci bot.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants