Skip to content
This repository has been archived by the owner on Oct 14, 2024. It is now read-only.

Commit

Permalink
refactor: code inspection improvements (#1095)
Browse files Browse the repository at this point in the history
* refactor: fix variable collides with imported package name

* refactor: type can be omitted

* refactor: fix grammar

* refactor: fix typos
  • Loading branch information
paralta authored Jan 16, 2024
1 parent a24aac3 commit 9aa03a8
Show file tree
Hide file tree
Showing 29 changed files with 48 additions and 49 deletions.
2 changes: 1 addition & 1 deletion .github/settings.yml
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ branches:
teams: []
# Required. Require status checks to pass before merging. Set to null to disable
required_status_checks:
# Required. Require branches to be up to date before merging.
# Required. Require branches to be up-to-date before merging.
strict: true
# Required. The list of status checks to require in order to merge into this branch
contexts: []
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/reusable-verification.yml
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ jobs:

- name: Vendor dependencies to retrieve licenses locally
# Vendor deps before running https://github.com/goph/licensei
# to avoid false-positives when modules Github repo could not be determined
# to avoid false-positives when modules GitHub repo could not be determined
run: go mod vendor

- name: Check licenses
Expand Down
6 changes: 3 additions & 3 deletions ARCHITECTURE.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ The VMClarity control plane includes several microservices:

- **Orchestrator**: Orchestrates and manages the life cycle of VMClarity
scan configs, scans and asset scans. Within the Orchestrator there is a
pluggable "provider" which connects the orchstrator to the environment to be
pluggable "provider" which connects the orchestrator to the environment to be
scanned and abstracts asset discovery, VM snapshotting as well as creation of
the scanner VMs. (**Note** The only supported provider today is AWS, other
hyperscalers are on the roadmap)
Expand All @@ -28,7 +28,7 @@ The VMClarity control plane includes several microservices:
CLI to offload work that would need to be done in every scanner, for example
downloading the latest vulnerability or malware signatures from the various DB
sources. The components included today are:
- grype-server: A rest API wrapper around the grype vulnerbility scanner
- grype-server: A rest API wrapper around the grype vulnerability scanner
- trivy-server: Trivy vulnerability scanner server
- exploitDB server: A test API which wraps the Exploit DB CVE to exploit mapping logic
- freshclam-mirror: A mirror of the ClamAV malware signatures
Expand All @@ -39,7 +39,7 @@ exporting the results to VMClarity API.

These components are containerized and can be deployed in a number of different
ways. For example our cloudformation installer deploys VMClarity on a VM using
docker in an dedicated AWS Virtual Private Cloud (VPC).
docker in a dedicated AWS Virtual Private Cloud (VPC).

Once the VMClarity server instance has been deployed, and the scan
configurations have been created, VMClarity will discover VM resources within
Expand Down
4 changes: 2 additions & 2 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,13 @@ possible, and, if possible, a test case.
### Building VMClarity Binaries

Makefile targets are provided to compile and build the VMClarity binaries.
`make build` can be used to build all of the components, but also specific
`make build` can be used to build all the components, but also specific
targets are provided, for example `make build-cli` and `make build-backend` to
build the specific components in isolation.

### Building VMClarity Containers

`make docker` can be used to build the VMClarity containers for all of the
`make docker` can be used to build the VMClarity containers for all the
components. Specific targets for example `make docker-cli` and `make
docker-backend` are also provided.

Expand Down
6 changes: 3 additions & 3 deletions TOUR.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,13 @@ Figure 3. VMClarity Scan Setup Step 1

<img src="img/vmclarity-scan-setup-1.png" alt="VMClarity Scan Setup - Step 1" width="40%" height="40%" title="VMClarity Scan Setup Step 1" />

- In the "New scan config" wizard shown in Figure 4, follow the wizard steps to name the scan, and identify the AWS scope (region, VPC, security groups, etc). In the example shown in Figure 4, the AWS us-east-2 region, and a specific VPC were identied as well as a specific EC2 instance with the name "vmclarity-demo-vm".
- In the "New scan config" wizard shown in Figure 4, follow the wizard steps to name the scan, and identify the AWS scope (region, VPC, security groups, etc.). In the example shown in Figure 4, the AWS us-east-2 region, and a specific VPC were identified as well as a specific EC2 instance with the name "vmclarity-demo-vm".

Figure 4. VMClarity Scan Setup Step 2

<img src="img/vmclarity-scan-setup-2.png" alt="VMClarity Scan Setup - Step 2" width="40%" height="40%" title="VMClarity Scan Setup Step 2" />

- Next, identify all of the scan types you want enabled. As Figure 5 shows, all of the available scan types have been enabled.
- Next, identify all the scan types you want enabled. As Figure 5 shows, all the available scan types have been enabled.

Figure 5. VMClarity Scan Setup Step 3

Expand All @@ -34,7 +34,7 @@ Figure 6. VMClarity Scan Setup Step 4

<img src="img/vmclarity-scan-setup-4.png" alt="VMClarity Scan Setup - Step 4" width="40%" height="40%" title="VMClarity Scan Setup Step 4" />

- Once all of the scan setup steps have been entered, click on "Save".
- Once all the scan setup steps have been entered, click on "Save".

In the Scan Configurations tab, you will see the scan config listed as shown in Figure 7.

Expand Down
2 changes: 1 addition & 1 deletion docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
| `VMCLARITY_ORCHESTRATOR_APISERVER_ADDRESS` | **yes** | | | The URL for the _API Server_ used by the _Orchestrator_ to interact with the API. Example: `https://apiserver.example.com:8888/api` |
| `VMCLARITY_ORCHESTRATOR_HEALTHCHECK_ADDRESS` | | `:8082` | | Bind address to used by the _Orchestrator_ for `healthz` endpoint. Example: `localhost:8082` which will make the health endpoints be available at `localhost:8082/healthz/live` and `localhost:8082/healthz/ready`. |
| `VMCLARITY_ORCHESTRATOR_DISCOVERY_INTERVAL` | | `2m` | | How frequently the _Discovery_ perform discovery of _Assets_. |
| `VMCLARITY_ORCHESTRATOR_CONTROLLER_STARTUP_DELAY` | | `7s` | | The time interval to wait between cotroller startups. **Do NOT change this parameter unless you know what you are doing.** |
| `VMCLARITY_ORCHESTRATOR_CONTROLLER_STARTUP_DELAY` | | `7s` | | The time interval to wait between controller startups. **Do NOT change this parameter unless you know what you are doing.** |
| `VMCLARITY_ORCHESTRATOR_ASSETSCAN_WATCHER_POLL_PERIOD` | | `15s` | | How frequently poll the API for events related _AssetScan_ objects. |
| `VMCLARITY_ORCHESTRATOR_ASSETSCAN_WATCHER_RECONCILE_TIMEOUT` | | `5m` | | Time period for reconciling a _AssetScan_ event is allowed to run. |
| `VMCLARITY_ORCHESTRATOR_ASSETSCAN_WATCHER_ABORT_TIMEOUT` | | `10m` | | Time period to wait for the _Scanner_ to gracefully stop on-going scan for _AssetScan_ before setting the state of the AssetScan to `Failed`. |
Expand Down
6 changes: 3 additions & 3 deletions docs/test_e2e.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# End to End testing guide
# End-to-end testing guide

## Table of Contents

Expand All @@ -7,7 +7,7 @@
- [2. Update installation/aws/VMClarity.cfn](#2-update-installationawsvmclaritycfn)
- [3. Install VMClarity cloudformation](#3-install-vmclarity-cloudformation)
- [4. Ensure that VMClarity backend is working correctly](#4-ensure-that-vmclarity-backend-is-working-correctly)
- [Performing an end to end test](#performing-an-end-to-end-test)
- [Performing an end-to-end test](#performing-an-end-to-end-test)

## Installing a specific VMClarity build on AWS

Expand Down Expand Up @@ -37,7 +37,7 @@ DOCKER_REGISTRY=<your docker registry> make push-docker
sudo journalctl -u vmclarity
```

## Performing an end to end test
## Performing an end-to-end test

1. Copy the example [scanConfig.json](scanConfig.json) into the ubuntu user's home directory

Expand Down
4 changes: 2 additions & 2 deletions e2e/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ import (
)

const (
DefaultScope string = "assetInfo/labels/any(t: t/key eq 'scanconfig' and t/value eq 'test')"
DefaultTimeout time.Duration = 60 * time.Second
DefaultScope string = "assetInfo/labels/any(t: t/key eq 'scanconfig' and t/value eq 'test')"
DefaultTimeout = 60 * time.Second
)

var FullScanFamiliesConfig = models.ScanFamiliesConfig{
Expand Down
2 changes: 1 addition & 1 deletion installation/kubernetes/helm/vmclarity/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ secrets.
| apiserver.image.digest | string | `""` | API Server image digest. If set will override the tag. |
| apiserver.image.pullPolicy | string | `"IfNotPresent"` | API Server image pull policy |
| apiserver.image.registry | string | `"ghcr.io"` | API Server image registry |
| apiserver.image.repository | string | `"openclarity/vmclarity-apiserver"` | API Server image repositiory |
| apiserver.image.repository | string | `"openclarity/vmclarity-apiserver"` | API Server image repository |
| apiserver.image.tag | string | `"latest"` | API Server image tag (immutable tags are recommended) |
| apiserver.logLevel | string | `"info"` | API Server log level |
| apiserver.podSecurityContext.enabled | bool | `true` | Pod security context enabled |
Expand Down
2 changes: 1 addition & 1 deletion installation/kubernetes/helm/vmclarity/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ apiserver:
image:
# -- API Server image registry
registry: ghcr.io
# -- API Server image repositiory
# -- API Server image repository
repository: openclarity/vmclarity-apiserver
# -- API Server image tag (immutable tags are recommended)
tag: latest
Expand Down
2 changes: 1 addition & 1 deletion pkg/apiserver/database/odatasql/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ func buildSelectFieldsFromSelectAndExpand(sqlVariant jsonsql.Variant, schemaMeta
var selectQuery *godata.GoDataSelectQuery
if selectString != nil && *selectString != "" {
// NOTE(sambetts):
// For now we'll won't parse the data here and instead pass
// For now we won't parse the data here and instead pass
// just the raw value into the selectTree. The select tree will
// parse the select query using the ExpandParser. If we can
// update the GoData select parser to handle paths properly and
Expand Down
2 changes: 1 addition & 1 deletion pkg/apiserver/database/odatasql/query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ type Options struct {

// Example field of a collection of primitive type inside of a nested
// complex type
OtherThings []string `json:OtherThings`
OtherThings []string `json:"OtherThings"`
}

type Engine struct {
Expand Down
2 changes: 1 addition & 1 deletion pkg/apiserver/rest/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func (s *ServerImpl) GetOpenAPISpec(ctx echo.Context) error {
// swagger-ui service dynamically loads the OpenAPI spec from the
// APIServer and knows where to send the TryItNow requests.
// Wherever the API server is accessed from through a proxy or
// sub-domain this will correct the servers entry to match that clients
// subdomain this will correct the servers entry to match that clients
// access path.
headers := ctx.Request().Header

Expand Down
2 changes: 1 addition & 1 deletion pkg/containerruntimediscovery/discoverer.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ type Discoverer interface {

type DiscovererFactory func(ctx context.Context) (Discoverer, error)

var discovererFactories map[string]DiscovererFactory = map[string]DiscovererFactory{
var discovererFactories = map[string]DiscovererFactory{
"docker": NewDockerDiscoverer,
"containerd": NewContainerdDiscoverer,
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/orchestrator/common/queue.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ type Queue[T ReconcileEvent] struct {
queue []T

// A map used as a set of unique items which are in the queue. This is
// used by Enqueue and Has to provide a quick reference to whats in the
// used by Enqueue and Has to provide a quick reference to what is in the
// queue without needing to loop through the queue slice.
inqueue map[string]struct{}

Expand Down Expand Up @@ -118,7 +118,7 @@ func (q *Queue[T]) Dequeue(ctx context.Context) (T, error) {
return item, nil
}

// Enqueue will add item to the queue if its not in the queue already.
// Enqueue will add item to the queue if it is not in the queue already.
func (q *Queue[T]) Enqueue(item T) {
q.l.Lock()
defer q.l.Unlock()
Expand Down
4 changes: 2 additions & 2 deletions pkg/orchestrator/provider/azure/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,15 @@ func handleAzureRequestError(err error, actionTmpl string, parts ...interface{})

var respError *azcore.ResponseError
if !errors.As(err, &respError) {
// Error should be a azcore.ResponseError otherwise something
// Error should be an azcore.ResponseError otherwise something
// bad has happened in the client.
return false, provider.FatalErrorf("unexpected error from azure while %s: %w", action, err)
}

sc := respError.StatusCode
switch {
case sc >= 400 && sc < 500:
// Client errors (BadRequest/Unauthorized etc) are Fatal. We
// Client errors (BadRequest/Unauthorized/etc.) are Fatal. We
// also return true to indicate we have NotFound which is a
// special case in a lot of processing.
return sc == http.StatusNotFound, provider.FatalErrorf("error from azure while %s: %w", action, err)
Expand Down
2 changes: 1 addition & 1 deletion pkg/orchestrator/provider/gcp/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func handleGcpRequestError(err error, actionTmpl string, parts ...interface{}) (
sc := gAPIError.Code
switch {
case sc >= http.StatusBadRequest && sc < http.StatusInternalServerError:
// Client errors (BadRequest/Unauthorized etc) are Fatal. We
// Client errors (BadRequest/Unauthorized/etc.) are Fatal. We
// also return true to indicate we have NotFound which is a
// special case in a lot of processing.
return sc == http.StatusNotFound, provider.FatalErrorf("error from gcp while %s: %w", action, gAPIError)
Expand Down
2 changes: 1 addition & 1 deletion pkg/orchestrator/provider/kubernetes/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import (
"github.com/openclarity/vmclarity/pkg/shared/utils"
)

var crDiscovererLabels map[string]string = map[string]string{
var crDiscovererLabels = map[string]string{
"app.kubernetes.io/component": "cr-discovery-server",
"app.kubernetes.io/name": "vmclarity",
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/shared/analyzer/purl.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,18 +124,18 @@ func purlStringToStruct(purlInput string) purl {
} else if len(purlParts) == 2 {
// Check type is a valid PURL type, if it is then use it.
//
// Otherwise check if the type is a namespace we know belong to
// Otherwise, check if the type is a namespace we know belong to
// one of the types, sometimes the anaylzers goof up and forget
// the type part, looking at you syft....
//
// If its a recognised namespace then we can correct the PURL,
// If it is a recognised namespace then we can correct the PURL,
// otherwise this is an invalid PURL so return the empty purl
// struct.
if isValidPurlType(purlParts[0]) {
output.typ = purlParts[0]
} else {
// Fix known cases otherwise just exclude the type from
// the purl or error maybe, havn't decided
// the purl or error maybe, haven't decided
switch purlParts[0] {
case "alpine":
output.typ = "apk"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -525,7 +525,7 @@ func TestReportParser_scanLynisReportFile(t *testing.T) {
}
}

var testdataMisconfigurations []types.Misconfiguration = []types.Misconfiguration{
var testdataMisconfigurations = []types.Misconfiguration{
{
ScannedPath: "scanPath",
TestCategory: "security",
Expand Down
2 changes: 1 addition & 1 deletion pkg/shared/log/log.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ const (
func InitLogger(level string, output io.Writer) {
logLevel, err := log.ParseLevel(level)
if err != nil {
log.Errorf("failed to prase log level, using default(%s): %v", DefaultLogLevel, err)
log.Errorf("failed to parse log level, using default(%s): %v", DefaultLogLevel, err)
logLevel = DefaultLogLevel
}
log.SetLevel(logLevel)
Expand Down
6 changes: 3 additions & 3 deletions pkg/shared/scanner/grype/local_grype.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ func (s *LocalScanner) run(sourceType utils.SourceType, userInput string) {
}
s.logger.Infof("Loading DB. update=%v", s.config.UpdateDB)

store, dbStatus, _, err := grype.LoadVulnerabilityDB(dbConfig, s.config.UpdateDB)
vulnerabilityStore, dbStatus, _, err := grype.LoadVulnerabilityDB(dbConfig, s.config.UpdateDB)

if err = validateDBLoad(err, dbStatus); err != nil {
ReportError(s.resultChan, fmt.Errorf("failed to load vulnerability DB: %w", err), s.logger)
Expand Down Expand Up @@ -116,7 +116,7 @@ func (s *LocalScanner) run(sourceType utils.SourceType, userInput string) {

s.logger.Infof("Found %d packages", len(packages))

vulnerabilityMatcher := createVulnerabilityMatcher(store)
vulnerabilityMatcher := createVulnerabilityMatcher(vulnerabilityStore)
allMatches, ignoredMatches, err := vulnerabilityMatcher.FindMatches(packages, context)
// We can ignore ErrAboveSeverityThreshold since we are not setting the FailSeverity on the matcher.
if err != nil && !errors.Is(err, grypeerr.ErrAboveSeverityThreshold) {
Expand All @@ -125,7 +125,7 @@ func (s *LocalScanner) run(sourceType utils.SourceType, userInput string) {
}

s.logger.Infof("Found %d vulnerabilities", len(allMatches.Sorted()))
doc, err := grype_models.NewDocument(packages, context, *allMatches, ignoredMatches, store.MetadataProvider, nil, dbStatus)
doc, err := grype_models.NewDocument(packages, context, *allMatches, ignoredMatches, vulnerabilityStore.MetadataProvider, nil, dbStatus)
if err != nil {
ReportError(s.resultChan, fmt.Errorf("failed to create document: %w", err), s.logger)
return
Expand Down
2 changes: 1 addition & 1 deletion pkg/shared/scanner/merge.go
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ func toVulnerabilityByKey(matches Matches) map[VulnerabilityKey]Vulnerability {
vulB, _ := json.Marshal(vul)
// nolint:errchkjson
newVulB, _ := json.Marshal(match.Vulnerability)
log.Debugf("Existing vul with the same key %q. vul=%+s, newVul=%s", key, vulB, newVulB)
log.Debugf("Existing vul with the same key %q. vul=%s, newVul=%s", key, vulB, newVulB)
} else if diff != nil {
log.Debugf("Existing vul with the same key %q. diff.JSONDiff=%+v", key, diff.JSONDiff)
} else {
Expand Down
3 changes: 1 addition & 2 deletions pkg/shared/scanner/trivy/scanner.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,7 @@ func New(c job_manager.IsConfig,

// Init trivy's loggers with a hook into our logger
lc := logrusCore{logger}
zap := zap.New(lc)
trivyLog.Logger = zap.Sugar()
trivyLog.Logger = zap.New(lc).Sugar()
dlog.SetLogger(trivyLog.Logger)
flog.SetLogger(trivyLog.Logger)

Expand Down
2 changes: 1 addition & 1 deletion pkg/shared/utils/containerrootfs/totempdirectory.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func toTempDirectory(ctx context.Context, src string) (Rootfs, error) {
successful := false
tdrf := &tempDirRootfs{}
defer func() {
// If we're successful then its the responsibility of the caller
// If we're successful then it is the responsibility of the caller
// to defer the cleanup, if we error during this function then
// we need to handle it.
if !successful {
Expand Down
Loading

0 comments on commit 9aa03a8

Please sign in to comment.