Skip to content

refactor: lazy configuration of oci auth and signature verification secrets#168

Merged
ahmad-ibra merged 14 commits intomainfrom
feat/oci-direct-auth
Aug 15, 2024
Merged

refactor: lazy configuration of oci auth and signature verification secrets#168
ahmad-ibra merged 14 commits intomainfrom
feat/oci-direct-auth

Conversation

@ahmad-ibra
Copy link
Member

@ahmad-ibra ahmad-ibra commented Aug 14, 2024

Description

Previously we were required to configure auth and signature verification secrets before we even add any oci rules. Now we can lazily create the auth and signature verification secrets as we create the rules. ie no need to prep all your pubkeys and creds before you even set up your rule.

This PR also does the following:

  • Refactors readOciPluginRules to follow the same pattern as all the other plugins.
    • This change addresses an issue with --reconfigureing oci rules. Previously rule updates werent getting properly persisted
  • Refactors integration tests
    • It ensures that we actually run the validator install --apply tests which should really bump our code coverage up by a lot
    • Covers the case of provisioning a new kind cluster or using a pre-provisioned cluster
  • Fixes a bug where validator install --apply was no longer working due to the kind cluster not starting up

Context on the integration test refactor

The reason for the big changes in the integration tests were that i had noticed the oci plugin integration tests were passing without me updating any of the prompts when they clearly shouldnt pass. This got me down a rabbit hole of investigating why they were passing and eventually making the necessary changes. While doing this, it uncovered a few other issues. For the sake of not adding even more to this PR, i've marked some TODOs around things that need to be fixed. IMO we should fix them in follow ups shortly after this PR is eventually merged.

Out of scope follow up work

  1. Add support for oci auth being configured with no secrets. This will allow us to run validator rules check on a private oci registry
  2. Fix and re-enable maas integration tests
  3. Fix the minor UX issues noted for the vsphere plugin values

@ahmad-ibra ahmad-ibra force-pushed the feat/oci-direct-auth branch from b516ff8 to 9ca49df Compare August 14, 2024 18:41
@ahmad-ibra ahmad-ibra force-pushed the feat/oci-direct-auth branch from 9ca49df to affc05e Compare August 14, 2024 18:42
@codecov
Copy link

codecov bot commented Aug 14, 2024

Codecov Report

Attention: Patch coverage is 59.34959% with 50 lines in your changes missing coverage. Please review.

Files Patch % Lines
pkg/services/validator/oci.go 53.19% 28 Missing and 16 partials ⚠️
pkg/cmd/validator/validator.go 33.33% 2 Missing and 2 partials ⚠️
...integration/_validator/testcases/test_validator.go 91.30% 0 Missing and 2 partials ⚠️
@@            Coverage Diff             @@
##             main     #168      +/-   ##
==========================================
+ Coverage   53.70%   53.77%   +0.07%     
==========================================
  Files          43       43              
  Lines        6100     6086      -14     
==========================================
- Hits         3276     3273       -3     
+ Misses       2003     1994       -9     
+ Partials      821      819       -2     
Files Coverage Δ
pkg/config/constants.go 100.00% <ø> (ø)
...integration/_validator/testcases/test_validator.go 93.59% <91.30%> (-0.05%) ⬇️
pkg/cmd/validator/validator.go 58.75% <33.33%> (-0.29%) ⬇️
pkg/services/validator/oci.go 50.79% <53.19%> (+2.85%) ⬆️

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 0912f6e...12a0e48. Read the comment docs.

@ahmad-ibra ahmad-ibra changed the title refactor: lazy configure of oci auth and signature verification secrets refactor: lazy configuration of oci auth and signature verification secrets Aug 15, 2024
@ahmad-ibra ahmad-ibra marked this pull request as ready for review August 15, 2024 13:24
@ahmad-ibra ahmad-ibra requested a review from a team as a code owner August 15, 2024 13:24
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. refactoring Refactoring / tech debt labels Aug 15, 2024
@TylerGillson
Copy link
Member

Regarding the three out of scope follow ups, I think this PR should address the first one:

  • Add support for oci auth being configured with no secrets. This will allow us to run validator rules check on a private oci registry

And I think the second two are inapplicable once you export DISABLE_KIND_CLUSTER_CHECK=true.

@TylerGillson
Copy link
Member

Regarding the integration test refactor and this comment:

It ensures that we actually run the validator install --apply tests which should really bump our code coverage up by a lot

I don't think that any refactor is called for. The testInstallSilentWait test already executes validator install -f config.yaml --apply --wait, so these changes actually won't impact coverage.

@ahmad-ibra ahmad-ibra force-pushed the feat/oci-direct-auth branch from fa51f16 to cb9c274 Compare August 15, 2024 19:06
@ahmad-ibra ahmad-ibra force-pushed the feat/oci-direct-auth branch from cb9c274 to c9081b9 Compare August 15, 2024 19:36
@ahmad-ibra ahmad-ibra force-pushed the feat/oci-direct-auth branch from 8257ee8 to ed8e43b Compare August 15, 2024 19:53
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Aug 15, 2024
@ahmad-ibra ahmad-ibra merged commit cc2c056 into main Aug 15, 2024
@ahmad-ibra ahmad-ibra deleted the feat/oci-direct-auth branch August 15, 2024 22:16
ahmad-ibra pushed a commit that referenced this pull request Aug 19, 2024
🤖 I have created a release *beep* *boop*
---


##
[0.1.2](v0.1.1...v0.1.2)
(2024-08-19)


### Features

* add maas plugin
([#160](#160))
([ab9f21a](ab9f21a))
* allow selecting aws creds from filesystem
([#171](#171))
([c3a714c](c3a714c))
* allow specifying Azure cloud to connect to
([#170](#170))
([6a4a704](6a4a704))
* read vCenter privileges from local file or editor
([#152](#152))
([94ddd90](94ddd90))
* set exit code 2 on validation failure; restore debug log file
([#150](#150))
([2a3fe4d](2a3fe4d))
* support configuring oci validationType on a rule
([#161](#161))
([8dfc501](8dfc501))
* support direct oci validation of private registries
([#173](#173))
([9cfeab9](9cfeab9))


### Bug Fixes

* correct TUI flow for `validator install -o --apply`
([#169](#169))
([0912f6e](0912f6e))
* export creds for aws and azure direct check
([#167](#167))
([5d569de](5d569de))


### Dependency Updates

* **deps:** update anchore/sbom-action action to v0.17.1
([#163](#163))
([416d23c](416d23c))
* **deps:** update github.com/validator-labs/validator-plugin-azure
digest to b4687e5
([#149](#149))
([e7ab9a6](e7ab9a6))
* **deps:** update github.com/validator-labs/validator-plugin-vsphere
digest to a93cb70
([#147](#147))
([79304b9](79304b9))
* **deps:** update module github.com/vmware/govmomi to v0.40.0
([#162](#162))
([acf4a25](acf4a25))


### Refactoring

* lazy configuration of oci auth and signature verification secrets
([#168](#168))
([cc2c056](cc2c056))
* remove explicit TypeMetas; use vapi constants
([#154](#154))
([28b321c](28b321c))

---
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:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants