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

refactor(repo): Simplify structure #308

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
16 changes: 0 additions & 16 deletions ARCHITECTURE.md

This file was deleted.

14 changes: 12 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,18 @@

_trivy-checks_ contains misconfiguration checks for Trivy

Please see [ARCHITECTURE.md](ARCHITECTURE.md) for more information.

_trivy-checks_ is an [Aqua Security](https://aquasec.com) open source project.
Learn about our open source work and portfolio [here](https://www.aquasec.com/products/open-source-projects/).
Join the community, and talk to us about any matter in [GitHub Discussion](https://github.com/aquasecurity/trivy/discussions).

## Project Layout

The directory structure is broken down as follows:

- `cmd/` - These CLI tools are primarily used during development for end-to-end testing without requiring the use of a library.
- `cmd/id` - This command helps generate the next available ID that is free when writing a new check.
- `checks` - All the checks are defined in this directory.
- `commands` - All Node-collector commands are defined in this directory.
- `compliance/` - Standaridized compliance specs such as CIS.
- `test` - Integration tests and other high-level tests that require a full build of the project.
- `scripts` - Useful generation scripts for bundle generation and verification purposes.
13 changes: 13 additions & 0 deletions checks/kubernetes/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,19 @@ wget https://github.com/aquasecurity/trivy-checks/raw/main/test/testdata/kuberne
conftest test denied.yaml --policy myPolicy/ --namespace builtin.kubernetes.KSV008
```

# Kubernetes checks classification
There are several Kubernetes checks that target various subsystems. They are loosely classified as follows.

| Target | Description |
|----------------|------------------------------------------------------------------------------------------------------------------------------------|
| Network | Checks primarily targeting the networking stack |
| Dynamic | Checks that evaluate deprecated and removed APIs |
Copy link
Contributor

Choose a reason for hiding this comment

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

either the description is not accurate or the dir name is bad. I think the intention was that these checks rely on environmental context for evaluation. for example, the deprecated API checks rely on information of which k8s version the user is running/evaluating against in order to decide if outdated or not.

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree the name isn't very meaningful but then again I didn't want to change it in this PR as there maybe users who rely on this structure.

I couldn't come up with a better description, so I'm open to ideas. Would the following work based on what you said?

"Checks whose decision outcome is dynamically determined based on the environmental context."

Copy link
Contributor

Choose a reason for hiding this comment

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

do you think the structure of checks could break users? I mean aren't the checks save/loaded from embedded/bundle in their complete hierarchy? is changing the internal hierachy problemattic?

about the description, it's something like:
"Checks that cannot evaluate based on input alone, and depend on environmental or user-provided context"

| CIS Benchmarks | Checks that are recommended by the CIS Benchmarks. The checks inside are targeted per each subsystem (e.g. apiserver, cni, etc.) |
Copy link
Contributor

Choose a reason for hiding this comment

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

If this dir is serving only CIS, and CIS is using only this dir then fine, but I will challange us abit here: aren't these checks valid k8s checks also outside CIS context? aren't CIS reports include k8s checks besides these checks (i presume something like non privileged pod check).

Copy link
Member Author

Choose a reason for hiding this comment

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

You're right in the sense that these checks are ordinary checks even outside of CIS context.

I'm open to re-organizing all k8s checks in general (maybe based on another type) but we just need to confirm first with the stakeholders that this won't cause any breakage as in the past some users did rely on the structure. Can we do this in another PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes I think we need to reorganize the checks which is part of the motivation for this PR. sure if you think it needs another pr that's fine

| Advanced | Checks that are recommended for the advanced uesrs of Kubernetes |
Copy link
Contributor

Choose a reason for hiding this comment

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

how do we define advanced users

| GKE | Checks specific to Google Kubernetes Engine |
| PSS | Checks pertaining to Pod Security Standards |
Copy link
Contributor

Choose a reason for hiding this comment

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

same comment as CIS



# Standards and best practices
This GitHub repository has controls that cover both [PodSecurityPolicy](https://kubernetes.io/docs/concepts/policy/pod-security-policy/) (PSP) and the Kubernetes [Pod Security Standards](https://kubernetes.io/docs/concepts/security/pod-security-standards/) (PSS), plus additional best practices.

Expand Down
5 changes: 1 addition & 4 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ module github.com/aquasecurity/trivy-checks
go 1.23.4

require (
github.com/aquasecurity/trivy v0.58.1-0.20250109050215-f9a6a7192722
github.com/aquasecurity/trivy v0.58.1-0.20250111034400-243e5a3af99e
github.com/aws-cloudformation/rain v1.21.0
github.com/hashicorp/hcl/v2 v2.23.0
github.com/liamg/iamgo v0.0.9
Expand Down Expand Up @@ -407,6 +407,3 @@ require (
tags.cncf.io/container-device-interface v0.8.0 // indirect
tags.cncf.io/container-device-interface/specs-go v0.8.0 // indirect
)

// needed until the new name is used everywhere
replace github.com/aquasecurity/trivy-policies v0.10.0 => github.com/aquasecurity/trivy-checks v0.10.2-0.20240417031955-932169bbd75f
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -355,8 +355,8 @@ github.com/aquasecurity/testdocker v0.0.0-20240730042311-4642e94c7fc8 h1:b43UVqY
github.com/aquasecurity/testdocker v0.0.0-20240730042311-4642e94c7fc8/go.mod h1:wXA9k3uuaxY3yu7gxrxZDPo/04FEMJtwyecdAlYrEIo=
github.com/aquasecurity/tml v0.6.1 h1:y2ZlGSfrhnn7t4ZJ/0rotuH+v5Jgv6BDDO5jB6A9gwo=
github.com/aquasecurity/tml v0.6.1/go.mod h1:OnYMWY5lvI9ejU7yH9LCberWaaTBW7hBFsITiIMY2yY=
github.com/aquasecurity/trivy v0.58.1-0.20250109050215-f9a6a7192722 h1:+YTyne1Ge2I8bpbqkClvMKCdK0U1qmQySjlS3cC4ih4=
github.com/aquasecurity/trivy v0.58.1-0.20250109050215-f9a6a7192722/go.mod h1:pDTEekX8yWaSI094UO3dTBnwI2xSZhwekFCbTSog8Jk=
github.com/aquasecurity/trivy v0.58.1-0.20250111034400-243e5a3af99e h1:d5JKS4O/5IO7lPweDsoH3DaEe3F96mMHwK9FqZeC2WY=
github.com/aquasecurity/trivy v0.58.1-0.20250111034400-243e5a3af99e/go.mod h1:pDTEekX8yWaSI094UO3dTBnwI2xSZhwekFCbTSog8Jk=
github.com/aquasecurity/trivy-db v0.0.0-20241209111357-8c398f13db0e h1:O5j5SeCNBrXApgBTOobO06q4LMxJxIhcSGE7H6Y154E=
github.com/aquasecurity/trivy-db v0.0.0-20241209111357-8c398f13db0e/go.mod h1:gS8VhlNxhraiq60BBnJw9kGtjeMspQ9E8pX24jCL4jg=
github.com/aquasecurity/trivy-java-db v0.0.0-20240109071736-184bd7481d48 h1:JVgBIuIYbwG+ekC5lUHUpGJboPYiCcxiz06RCtz8neI=
Expand Down
File renamed without changes.
30 changes: 23 additions & 7 deletions pkg/specs/loader.go → pkg/compliance/loader.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package specs
package compliance

import (
"embed"
Expand All @@ -10,24 +10,40 @@ import (
"gopkg.in/yaml.v3"
)

const ComplianceFolder = "compliance"
// Loader access compliance specs
type Loader interface {
GetSpecByName(name string) string
}

type specLoader struct {
}

// NewSpecLoader instansiate spec loader
func NewSpecLoader() Loader {
return &specLoader{}
}

// GetSpecByName get spec name and return spec data
func (sl specLoader) GetSpecByName(name string) string {
return GetSpec(name)
}

var (
//go:embed compliance
complainceFS embed.FS
//go:embed *.yaml
complianceFS embed.FS
)

var complianceSpecMap map[string]string

// Load compliance specs
func init() {
dir, _ := complainceFS.ReadDir(ComplianceFolder)
complianceSpecMap = make(map[string]string, 0)
dir, _ := complianceFS.ReadDir(".")
complianceSpecMap = make(map[string]string)
for _, r := range dir {
if !strings.Contains(r.Name(), ".yaml") {
continue
}
file, err := complainceFS.Open(fmt.Sprintf("%s/%s", ComplianceFolder, r.Name()))
file, err := complianceFS.Open(fmt.Sprintf("%s", r.Name()))
if err != nil {
panic(err)
}
Expand Down
20 changes: 10 additions & 10 deletions pkg/specs/loader_test.go → pkg/compliance/loader_test.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package specs
package compliance

import (
"os"
Expand All @@ -13,14 +13,14 @@ func TestLoadSpecs(t *testing.T) {
specName string
wantSpecPath string
}{
{name: "nsa spec", specName: "k8s-nsa-1.0", wantSpecPath: "./compliance/k8s-nsa-1.0.yaml"},
{name: "k8s cis bench", specName: "k8s-cis-1.23", wantSpecPath: "./compliance/k8s-cis-1.23.yaml"},
{name: "k8s pss baseline", specName: "k8s-pss-baseline-0.1", wantSpecPath: "./compliance/k8s-pss-baseline-0.1.yaml"},
{name: "k8s pss restricted", specName: "k8s-pss-restricted-0.1", wantSpecPath: "./compliance/k8s-pss-restricted-0.1.yaml"},
{name: "awscis1.2", specName: "aws-cis-1.2", wantSpecPath: "./compliance/aws-cis-1.2.yaml"},
{name: "awscis1.4", specName: "aws-cis-1.4", wantSpecPath: "./compliance/aws-cis-1.4.yaml"},
{name: "docker cis bench", specName: "docker-cis-1.6.0", wantSpecPath: "./compliance/docker-cis-1.6.0.yaml"},
{name: "awscis1.2 by filepath", specName: "@./compliance/aws-cis-1.2.yaml", wantSpecPath: "./compliance/aws-cis-1.2.yaml"},
{name: "nsa spec", specName: "k8s-nsa-1.0", wantSpecPath: "./k8s-nsa-1.0.yaml"},
{name: "k8s cis bench", specName: "k8s-cis-1.23", wantSpecPath: "./k8s-cis-1.23.yaml"},
{name: "k8s pss baseline", specName: "k8s-pss-baseline-0.1", wantSpecPath: "./k8s-pss-baseline-0.1.yaml"},
{name: "k8s pss restricted", specName: "k8s-pss-restricted-0.1", wantSpecPath: "./k8s-pss-restricted-0.1.yaml"},
{name: "awscis1.2", specName: "aws-cis-1.2", wantSpecPath: "./aws-cis-1.2.yaml"},
{name: "awscis1.4", specName: "aws-cis-1.4", wantSpecPath: "./aws-cis-1.4.yaml"},
{name: "docker cis bench", specName: "docker-cis-1.6.0", wantSpecPath: "./docker-cis-1.6.0.yaml"},
{name: "awscis1.2 by filepath", specName: "@./aws-cis-1.2.yaml", wantSpecPath: "./aws-cis-1.2.yaml"},
{name: "bogus spec", specName: "foobarbaz"},
}

Expand All @@ -30,7 +30,7 @@ func TestLoadSpecs(t *testing.T) {
wantSpecData, err := os.ReadFile(tt.wantSpecPath)
assert.NoError(t, err)
gotSpecData := GetSpec(tt.specName)
assert.Equal(t, gotSpecData, string(wantSpecData))
assert.Equal(t, string(wantSpecData), gotSpecData)
} else {
assert.Empty(t, GetSpec(tt.specName), tt.name)
}
Expand Down
18 changes: 0 additions & 18 deletions pkg/rules/rules.go

This file was deleted.

23 changes: 0 additions & 23 deletions pkg/spec/spec.go

This file was deleted.

Loading