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 --custom-resources flag to validator rules check as an alternate to -f #218

Merged
merged 9 commits into from
Sep 9, 2024

Conversation

ahmad-ibra
Copy link
Collaborator

@ahmad-ibra ahmad-ibra commented Sep 6, 2024

Issue

Resolves #178
Resolves #136

Description

This PR adds support for passing in custom resource documents to the validator rules check command via the --custom-resources flag. The --custom-resources flag can receive either a path to a directory or to a file.

File selection

If a directory path is provided, we check each file within the directory for CR documents. Note that we walk the entire file tree from a particular directory. This means that users are able to organize their CRs in a manner like this:

❯ tree tests/integration/_validator/testcases/data/validator-crs
tests/integration/_validator/testcases/data/validator-crs
├── combined.yaml
├── network
│   └── network.yaml
└── oci
    └── oci.yaml

3 directories, 3 files

In the above example, we'll look for CRs within the slice of all 3 files found (combined.yaml, network.yaml, and oci.yaml). Users have the flexibility to organize their CRs as they desire.

If a single file is instead passed in via the --cr flag, we only work with a slice containing the single file that was provided.

CR Unmarshalling

Given a slice of files (which may only contain a single file), we range over each file and attempt the following:

We try to read all the CR documents that are contained in that file and run validations against them.

For instance, if the path to the following file is passed in:

apiVersion: validation.spectrocloud.labs/v1alpha1
kind: OciValidator
metadata:
  name: oci-validator-combined-oci-rules
spec:
  ociRegistryRules:
    - name: "public oci registry with tag"
      host: "docker.io"
      validationType: "none"
      artifacts:
        - ref: "library/redis:7.2.4"
---
apiVersion: validation.spectrocloud.labs/v1alpha1
kind: NetworkValidator
metadata:
  name: network-validator-combined-network-rules
spec:
  dnsRules:
  - name: Resolve Google
    host: google.com

We'll run validations against both he network and the oci plugin rules that are defined in the file. Users can choose to store all CRs in a single file, or spread them across multiple files. Whatever configuration they choose will work.

Error handling

Succesfull validation can result in a lot of text being printed to a users terminal. Given this, I wanted to avoid "hiding" an error in the flood of validation results. If any CR document is invalid, an error message will be returned and no validations will run. It'll be obvious what failed and how to fix it.

Copy link

codecov bot commented Sep 6, 2024

Codecov Report

Attention: Patch coverage is 58.76289% with 40 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
pkg/cmd/validator/validator.go 53.42% 21 Missing and 13 partials ⚠️
pkg/utils/file/file.go 63.63% 2 Missing and 2 partials ⚠️
...integration/_validator/testcases/test_validator.go 75.00% 1 Missing and 1 partial ⚠️
@@            Coverage Diff             @@
##             main     #218      +/-   ##
==========================================
+ Coverage   53.33%   53.43%   +0.09%     
==========================================
  Files          45       46       +1     
  Lines        6272     6367      +95     
==========================================
+ Hits         3345     3402      +57     
- Misses       2082     2105      +23     
- Partials      845      860      +15     
Files with missing lines Coverage Δ
cmd/validator.go 82.60% <100.00%> (+1.27%) ⬆️
pkg/config/config.go 66.03% <ø> (ø)
pkg/config/constants.go 100.00% <ø> (ø)
...integration/_validator/testcases/test_validator.go 93.40% <75.00%> (-0.28%) ⬇️
pkg/utils/file/file.go 63.63% <63.63%> (ø)
pkg/cmd/validator/validator.go 60.07% <53.42%> (-0.77%) ⬇️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 97875f3...af7f421. Read the comment docs.

@ahmad-ibra ahmad-ibra marked this pull request as ready for review September 6, 2024 23:27
@ahmad-ibra ahmad-ibra requested a review from a team as a code owner September 6, 2024 23:27
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. new-feature Net-new feature labels Sep 6, 2024
Copy link
Member

@TylerGillson TylerGillson left a comment

Choose a reason for hiding this comment

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

Nice work! This is awesome. One tiny nit... sorry 😁

Can you please raise a follow up PR in the docs repo to cover this new usage?

cmd/validator.go Outdated Show resolved Hide resolved
Copy link
Member

@TylerGillson TylerGillson left a comment

Choose a reason for hiding this comment

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

I'm having naming hesitancy about --cr. Typically the long name for flags is not abbreviated. What do you think about --cr-path? Or --custom-resources?

@ahmad-ibra ahmad-ibra changed the title feat: add --cr flag to validator rules check as an alternate to -f feat: add --custom-resources flag to validator rules check as an alternate to -f Sep 9, 2024
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Sep 9, 2024
@ahmad-ibra ahmad-ibra merged commit 4aa14b6 into main Sep 9, 2024
8 checks passed
@ahmad-ibra ahmad-ibra deleted the feat/cr-flag branch September 9, 2024 15:39
ahmad-ibra added a commit to validator-labs/docs that referenced this pull request Sep 9, 2024
## Issue
N/A

## Description
Document the usage of the `--custom-resources` flag which was added
here:
validator-labs/validatorctl#218
TylerGillson added a commit that referenced this pull request Sep 11, 2024
🤖 I have created a release *beep* *boop*
---


##
[0.2.0](v0.1.4...v0.2.0)
(2024-09-11)


### ⚠ BREAKING CHANGES

* remove role privilege rules
([#217](#217))

### Features

* add --custom-resources flag to validator rules check as an alternate
to -f
([#218](#218))
([4aa14b6](4aa14b6))


### Bug Fixes

* add missing Host.Config.Storage privilege & document
([#212](#212))
([c7408a6](c7408a6))


### Docs

* remove subcommand docs from repo
([#205](#205))
([57ca6cb](57ca6cb))


### Dependency Updates

* **deps:** update golang.org/x/exp digest to e7e105d
([#214](#214))
([c4ac163](c4ac163))


### Refactoring

* remove role privilege rules
([#217](#217))
([b0976db](b0976db))
* update `executePlugins` function to operate on a slice of
`PluginSpec`'s
([#206](#206))
([97875f3](97875f3))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm This PR has been approved by a maintainer new-feature Net-new feature size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
2 participants