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

feat: Add report package, print reports to stdout #40

Merged
merged 22 commits into from
Jun 5, 2023
Merged

Conversation

viccuad
Copy link
Member

@viccuad viccuad commented May 26, 2023

Description

Partial implementation of #36.

feat: Add report package, print reports to stdout

Test

Manual for now.

Example:

output
$ ./bin/audit-scanner -k kubewarden --namespace default -l error | jq .
{
  "metadata": {
    "name": "polr-clusterwide",
    "creationTimestamp": null
  },
  "summary": {
    "pass": 0,
    "fail": 0,
    "warn": 0,
    "error": 0,
    "skip": 0
  }
}
{
  "metadata": {
    "name": "polr-ns-default",
    "namespace": "default",
    "creationTimestamp": "2023-05-25T14:27:40Z"
  },
  "scope": {},
  "summary": {
    "pass": 5,
    "fail": 1,
    "warn": 0,
    "error": 0,
    "skip": 0
  },
  "results": [
    {
      "source": "kubewarden",
      "policy": "cap-drop-capabilities",
      "timestamp": {
        "seconds": 1685024860,
        "nanos": 0
      },
      "result": "pass"
    },
    {
      "source": "kubewarden",
      "policy": "cap-no-privilege-escalation",
      "timestamp": {
        "seconds": 1685024860,
        "nanos": 0
      },
      "result": "pass"
    },
    {
      "source": "kubewarden",
      "policy": "cap-no-privileged-pod",
      "timestamp": {
        "seconds": 1685024860,
        "nanos": 0
      },
      "result": "fail",
      "message": "Privileged container is not allowed"
    },
    {
      "source": "kubewarden",
      "policy": "cap-do-not-run-as-root",
      "timestamp": {
        "seconds": 1685024860,
        "nanos": 0
      },
      "result": "pass"
    },
    {
      "source": "kubewarden",
      "policy": "cap-no-host-namespace-sharing",
      "timestamp": {
        "seconds": 1685024860,
        "nanos": 0
      },
      "result": "pass"
    },
    {
      "source": "kubewarden",
      "policy": "cap-do-not-share-host-paths",
      "timestamp": {
        "seconds": 1685024860,
        "nanos": 0
      },
      "result": "pass"
    }
  ]
}

Additional Information

Tradeoff

Potential improvement

Missing:

  • Add tests for report.Save().

viccuad added 2 commits May 25, 2023 15:09
Signed-off-by: Víctor Cuadrado Juan <vcuadradojuan@suse.de>
Signed-off-by: Víctor Cuadrado Juan <vcuadradojuan@suse.de>
@viccuad viccuad self-assigned this May 26, 2023
@viccuad viccuad force-pushed the add-report branch 2 times, most recently from bded591 to 2e28a19 Compare May 26, 2023 16:04
@viccuad viccuad marked this pull request as ready for review May 30, 2023 13:48
@viccuad viccuad requested a review from a team as a code owner May 30, 2023 13:49
@viccuad viccuad force-pushed the add-report branch 2 times, most recently from 8e4e3dd to 544b12c Compare May 30, 2023 15:24
Copy link
Member

@jvanz jvanz left a comment

Choose a reason for hiding this comment

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

Overall, it looks good to me. I think we will need to change how we do some things. But I think we will notice what needs to be improved in future steps on the scanner. Therefore, I think we can merge it and iterate over it during the next steps.

jvanz and others added 8 commits June 1, 2023 14:50
Due some memory usage issues this commit updates the golinter version
to solve the problem in some developers environments.

Signed-off-by: José Guilherme Vanz <jguilhermevanz@suse.com>
Signed-off-by: Víctor Cuadrado Juan <vcuadradojuan@suse.de>
This is needed for PolicyReport.Summary.Skip

Signed-off-by: Víctor Cuadrado Juan <vcuadradojuan@suse.de>
Signed-off-by: Víctor Cuadrado Juan <vcuadradojuan@suse.de>
Signed-off-by: Víctor Cuadrado Juan <vcuadradojuan@suse.de>
Signed-off-by: Víctor Cuadrado Juan <vcuadradojuan@suse.de>
This is needed as ReportResults.scope point to their namespace.

Signed-off-by: Víctor Cuadrado Juan <vcuadradojuan@suse.de>
Signed-off-by: Víctor Cuadrado Juan <vcuadradojuan@suse.de>
@viccuad
Copy link
Member Author

viccuad commented Jun 1, 2023

Force-pushed a new version (sorry), with partial history, since I refactored heavily everything.

Took the liberty to resolve conversations from the previous review, to try and short the thread.

viccuad added 2 commits June 1, 2023 15:46
Signed-off-by: Víctor Cuadrado Juan <vcuadradojuan@suse.de>
Enhance scanner{} with a report.PolicyStore interface. Create
policyStore, and fill it when auditing resources.

Signed-off-by: Víctor Cuadrado Juan <vcuadradojuan@suse.de>
@viccuad viccuad requested a review from flavio June 1, 2023 14:08
@viccuad viccuad requested a review from jvanz June 1, 2023 14:08
If `--policyServerFQDN` is not empty, query PolicyServers at
  `https://<FQDN>:3000/audit`
useful for out-of-cluster debugging.

Signed-off-by: Víctor Cuadrado Juan <vcuadradojuan@suse.de>
Copy link
Member

@flavio flavio left a comment

Choose a reason for hiding this comment

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

nice refactoring, I left some comments 👏 👍

viccuad added 4 commits June 2, 2023 18:07
Signed-off-by: Víctor Cuadrado Juan <vcuadradojuan@suse.de>
Signed-off-by: Víctor Cuadrado Juan <vcuadradojuan@suse.de>
Remove `report.go.RInterface`, `store.PolicyReportStore` interface.
Move `client.go.Save()` into store.go.

Signed-off-by: Víctor Cuadrado Juan <vcuadradojuan@suse.de>
Signed-off-by: Víctor Cuadrado Juan <vcuadradojuan@suse.de>
@viccuad viccuad requested a review from flavio June 2, 2023 16:20
@viccuad
Copy link
Member Author

viccuad commented Jun 2, 2023

I don't get why the unit tests are failing on CI, they pass locally. Will have a look at that next day.

Copy link
Member

@flavio flavio left a comment

Choose a reason for hiding this comment

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

I left some minor comments

viccuad added 3 commits June 5, 2023 11:22
Signed-off-by: Víctor Cuadrado Juan <vcuadradojuan@suse.de>
Signed-off-by: Víctor Cuadrado Juan <vcuadradojuan@suse.de>
Signed-off-by: Víctor Cuadrado Juan <vcuadradojuan@suse.de>
@viccuad viccuad requested a review from flavio June 5, 2023 09:29
@viccuad
Copy link
Member Author

viccuad commented Jun 5, 2023

I'm still puzzled on why the unit-tests fail without any error for the report package. They pass locally, and I'm running the same go version. The go env vars also look fine.

Copy link
Member

@flavio flavio left a comment

Choose a reason for hiding this comment

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

thanks for all the changes, LGTM

viccuad added 2 commits June 5, 2023 13:16
Signed-off-by: Víctor Cuadrado Juan <vcuadradojuan@suse.de>
Signed-off-by: Víctor Cuadrado Juan <vcuadradojuan@suse.de>
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for this docs! Pretty useful!

Copy link
Member

@jvanz jvanz 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 the patience! :)

@viccuad viccuad merged commit 7f48e2b into main Jun 5, 2023
@viccuad viccuad deleted the add-report branch June 5, 2023 12:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants