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: simplify helm prompts #115

Merged
merged 14 commits into from
Aug 1, 2024
Merged

refactor: simplify helm prompts #115

merged 14 commits into from
Aug 1, 2024

Conversation

ahmad-ibra
Copy link
Collaborator

@ahmad-ibra ahmad-ibra commented Jul 31, 2024

Issue

Resolves #100

Description

Add support for configuring helm via the new helmConfig and updated helmRelease structs from validator. This greatly reduces the amount of prompts users need to make their way through to actually deploy the validator with any plugin.

Tests

  • Deployed validator and plugins with the default helm and image registries.
  • Deployed validator and plugins on a private harbor registry

Out of scope

I've intentionally NOT implemented changes to handle PLT-1300. There are some TODO's littered throughout the codebase still that relate to that issue.

ahmad-ibra added a commit to validator-labs/validator that referenced this pull request Aug 1, 2024
…harts are installed (#359)

## Description
In validatorctl, when configuring a custom private registry, pulling the
plugin charts fails with this error:
```
2024-07-31 18:34:22 ERROR Plugin failed to install
                      └ validator-plugin-network: failed to pull chart: failed to fetch image from registry: GET https://toolbox.palette-adv.spectrocloud.com/v2/ahmad/charts/validator-plugin-network/validator-plugin-network/manifests/0.0.21: NOT_FOUND: repository ahmad/charts/validator-plugin-network/validator-plugin-network not found
```

As can be seen above, there is an extra `/validator-plugin-network`
tacked on at the end.
This PR fixes that.

This PR is a pre-requisite to
validator-labs/validatorctl#115

## Test Notes
Tested this by:
1. running `helm install validator chart/validator/ -f
chart/validator/values.yaml`
2. running `./bin/validator install` and configuring a plugin using the
default registry (quay)
3. running `./bin/validator install` and configuring a plugin using a
custom private registry (harbor)

Everything runs correctly with this change. Prior to this change, test 3
was failing.
ahmad-ibra added a commit to validator-labs/validator that referenced this pull request Aug 1, 2024
…harts are installed (#359)

## Description
In validatorctl, when configuring a custom private registry, pulling the
plugin charts fails with this error:
```
2024-07-31 18:34:22 ERROR Plugin failed to install
                      └ validator-plugin-network: failed to pull chart: failed to fetch image from registry: GET https://toolbox.palette-adv.spectrocloud.com/v2/ahmad/charts/validator-plugin-network/validator-plugin-network/manifests/0.0.21: NOT_FOUND: repository ahmad/charts/validator-plugin-network/validator-plugin-network not found
```

As can be seen above, there is an extra `/validator-plugin-network`
tacked on at the end.
This PR fixes that.

This PR is a pre-requisite to
validator-labs/validatorctl#115

## Test Notes
Tested this by:
1. running `helm install validator chart/validator/ -f
chart/validator/values.yaml`
2. running `./bin/validator install` and configuring a plugin using the
default registry (quay)
3. running `./bin/validator install` and configuring a plugin using a
custom private registry (harbor)

Everything runs correctly with this change. Prior to this change, test 3
was failing.
Copy link

codecov bot commented Aug 1, 2024

Codecov Report

Attention: Patch coverage is 63.52201% with 58 lines in your changes missing coverage. Please review.

Files Patch % Lines
pkg/services/validator/common.go 48.64% 13 Missing and 6 partials ⚠️
pkg/components/validator.go 28.57% 15 Missing ⚠️
pkg/cmd/validator/validator.go 50.00% 3 Missing and 2 partials ⚠️
pkg/components/vsphere.go 0.00% 4 Missing ⚠️
pkg/services/validator/validator_service.go 20.00% 1 Missing and 3 partials ⚠️
pkg/components/network.go 0.00% 3 Missing ⚠️
pkg/components/oci.go 0.00% 3 Missing ⚠️
pkg/services/validator/aws.go 0.00% 0 Missing and 1 partial ⚠️
pkg/services/validator/azure.go 0.00% 0 Missing and 1 partial ⚠️
pkg/services/validator/network.go 50.00% 0 Missing and 1 partial ⚠️
... and 2 more
@@            Coverage Diff             @@
##             main     #115      +/-   ##
==========================================
- Coverage   51.12%   51.05%   -0.07%     
==========================================
  Files          44       44              
  Lines        4937     4832     -105     
==========================================
- Hits         2524     2467      -57     
+ Misses       1751     1722      -29     
+ Partials      662      643      -19     
Files Coverage Δ
pkg/config/constants.go 100.00% <ø> (ø)
...integration/_validator/testcases/test_validator.go 94.50% <100.00%> (-0.34%) ⬇️
pkg/services/validator/aws.go 52.41% <0.00%> (ø)
pkg/services/validator/azure.go 44.09% <0.00%> (ø)
pkg/services/validator/network.go 47.38% <50.00%> (ø)
pkg/services/validator/oci.go 47.76% <0.00%> (ø)
pkg/services/validator/vmware.go 41.47% <0.00%> (ø)
pkg/components/network.go 0.00% <0.00%> (ø)
pkg/components/oci.go 0.00% <0.00%> (ø)
pkg/components/vsphere.go 0.00% <0.00%> (ø)
... and 4 more

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 0c4487f...7fa468d. Read the comment docs.

@ahmad-ibra ahmad-ibra marked this pull request as ready for review August 1, 2024 21:14
@ahmad-ibra ahmad-ibra requested a review from a team as a code owner August 1, 2024 21:14
@dosubot dosubot bot added size:XL This PR changes 500-999 lines, ignoring generated files. refactoring Refactoring / tech debt labels Aug 1, 2024
hack/validator.tmpl Outdated Show resolved Hide resolved
pkg/components/validator.go Outdated Show resolved Hide resolved
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Aug 1, 2024
@ahmad-ibra ahmad-ibra merged commit 8ce75a1 into main Aug 1, 2024
8 checks passed
@ahmad-ibra ahmad-ibra deleted the refactor/helm-config branch August 1, 2024 22:11
TylerGillson added a commit that referenced this pull request Aug 6, 2024
🤖 I have created a release *beep* *boop*
---


##
[0.1.0](v0.0.6...v0.1.0)
(2024-08-06)


### ⚠ BREAKING CHANGES

* split plugin rule configuration and installation into separate
commands
([#121](#121))

### Features

* add docs command; refactor to use embeddedfs pkg
([#116](#116))
([dbe19c5](dbe19c5))
* read CA certs for network rules, add HTTPFileRules, AMIRules
([#117](#117))
([0c4487f](0c4487f))
* support direct rule evaluation with `validator check --direct`
([#127](#127))
([f1fb0d6](f1fb0d6))


### Docs

* added subcommands docs page
([#110](#110))
([9fa23dc](9fa23dc))


### Dependency Updates

* **deps:** update github.com/validator-labs/validator-plugin-azure
digest to ba947e3
([#134](#134))
([2a1058d](2a1058d))
* **deps:** update github.com/validator-labs/validator-plugin-vsphere
digest to 9b1f05b
([#135](#135))
([253f328](253f328))


### Refactoring

* remove -s flag
([#126](#126))
([9373e02](9373e02))
* simplify helm prompts
([#115](#115))
([8ce75a1](8ce75a1))
* split plugin rule configuration and installation into separate
commands
([#121](#121))
([6eaee77](6eaee77))

---
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 refactoring Refactoring / tech debt size:XL This PR changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🎨 Simplify Helm registry prompts
2 participants