VLT091 plugin testing framework stepwise by catsby · Pull Request #9270 · hashicorp/vault

catsby · 2020-06-19T14:46:11Z

This PR adds a new plugin testing framework stepwise into the Vault SDK. This represents the core of the framework with environments and tests to follow shortly. This work was reviewed and approved to merge into this feature branch in #9166 , and is now being opened to be merged into the main branch.

The main changes/additions are summarized as:

Move helper/testhelpers/logical/testing.go to the Vault SDK package
Update all plugin tests that use the above referenced logicaltest package
Adds a new package stepwise, a spiritual successor to the logicaltest package

Future PRs will introduce:

An implementation of an stepwise.Environment; the first will be a Docker environment
Multiple plugin tests that utilize the stepwise framework and docker environment

Remaining:

resolve any merge conflicts
README for the stepwise package

* master: Sync Protobuf dependencies between core and sdk (#9154) Document and give an example of the input size limits when using the FF3-1 transform. (#9151) Fix feature flag persistence: we shouldn't have excluded dr primaries, they too must write feature flags. DR secondaries might not need depend on feature flags being there, but a DR primary could also be (or become) a perf primary. (#9148)

* master: changelog++ certutil/helpers.go: Allow 3072 RSA key sizes. (#8343)

* master: (36 commits) Minor transform docs rewording (#9223) Validate physical CockroachDB table config value before using it (#9191) Validate physical MySQL database and table config values before using them (#9189) add disable_iss_validation option to k8s auth docs (#9142) fix: configutil redeclared as imported package name (#9211) Integrate password policies into RabbitMQ secret engine (#9143) Clarify cache setting. (#9204) Test pre-1.4 seal migration (#9085) Simple typos (#9119) Update contribution guidelines changelog++ Add ssh signing algorithm as a role option. (#9096) replacing "a key usage mode" as it is confusing (#9194) changelog++ fix: invalidate cached clients after a config change in the aws secrets backend (#9186) website: remove whitepaper link from subnav (#9190) changelog++ Improving transit batch encrypt and decrypt latencies (#8775) changelog++ AWS: Add iam_groups parameter to role create/update (#8811) ...

* adding stepwise testing, but there are protocol buff error :/ * move file and update sdk/go.mo * update/sync modules * update from other branch * update sdk/go.mod * some cleanups after feedback * remove enviornments from this PR * update vendor * change from running go mod tidy * change from go mod tidy * Update sdk/testing/stepwise/helpers.go Co-authored-by: Michael Golowka <72365+pcman312@users.noreply.github.com> * Update sdk/testing/stepwise/helpers.go Co-authored-by: Michael Golowka <72365+pcman312@users.noreply.github.com> * change panic to error * Update sdk/testing/stepwise/helpers.go return `nil` and not `err` at the end Co-authored-by: Michael Golowka <72365+pcman312@users.noreply.github.com> * Defer close() on successful Open of a file * document the re-creation of steps * Update sdk/testing/stepwise/stepwise.go Co-authored-by: Michael Golowka <72365+pcman312@users.noreply.github.com> * remove unused BarrierKeys() * Update sdk/testing/stepwise/stepwise.go Co-authored-by: Michael Golowka <72365+pcman312@users.noreply.github.com> * updates from feedback * fix return with bad arguments * Rename things: - StepOperation -> Operation - StepwiseEnvironment -> Environment - StepCheckFunc -> AssertionFunc - step.Check -> step.Assert * document the environment interface methods * rename EnvironmentOptions to MountOptions * rename Name to RegistryName * remove ExpectError because it's redundant * minor doc update * Update sdk/testing/stepwise/stepwise.go Co-authored-by: Michael Golowka <72365+pcman312@users.noreply.github.com> * add checkShouldRun function * remove redundant return * remove vestigial PreCheck function * add tt.Helper() to makeRequest * minor code formatting and document 1-based index for log output of Steps Co-authored-by: Michael Golowka <72365+pcman312@users.noreply.github.com>

* master: (31 commits) changelog++ changelog++ Ui/replication status discoverability (#8705) Update CHANGELOG.md Counter that increments on every secret engine lease creation. (#9244) Add password_policy field to Azure docs (#9249) Replaced ClusterMetricSink's cluster name with an atomic.Value. (#9252) Fix database creds rotation panic for nil resp (#9258) changelog++ changelog++ Move sdk/helper/random -> helper/random (#9226) UI: Disallow kv2 with too large 'max versions' value (#9242) Allow mTLS for mysql secrets engine (#9181) docs: add sample revocation for mongodb (#9245) Add new Telemetry config options (#9238) Add a simple sealed gauge, updated when seal status changes (#9177) Test Shamir-to-Transit and Transit-to-Shamir Seal Migration for post-1.4 Vault. (#9214) Configure metrics wrapper with the "global" object, not just the fanout. (#9099) changelog++ Add backend type to audit logs (#9167) ...

calvn

Looking great so far, the comments in the PR made it easy to follow!

There are a few TODOs that got added in this PR, but I'm not sure whether they should be resolved before the merge or whether they are meant to stay.

calvn · 2020-06-19T17:50:16Z

+const pluginPrefix = "vault-plugin-"
+
+// CompilePlugin is a helper method to compile a source plugin
+// TODO refactor compile plugin input and output to be types


Should this TODO be addressed before merging?

I'm tracking this internally as I'm not 100% sure on the inputs/outputs as is, so I'm deferring this for a future PR.

Co-authored-by: Calvin Leung Huang <cleung2010@gmail.com>

* master: Fix to failing test compilation. (#9272) Added gauges to count KV secrets. (#9250)

calvn · 2020-06-19T23:09:12Z

+const pluginPrefix = "vault-plugin-"
+
+// CompilePlugin is a helper method to compile a source plugin
+// TODO refactor compile plugin input and output to be types


Since all non-error return values are strings, might be good to add a comment around them so consumers don't need to look into the source to figure this out.

remove dead line Co-authored-by: Alexander Bezobchuk <alexanderbez@users.noreply.github.com>

fix capitalization in code comment Co-authored-by: Alexander Bezobchuk <alexanderbez@users.noreply.github.com>

calvn

Thanks for extending the tests for stepwise, everything looks good! One small comment on updating the godoc for CompilePlugin but otherwise ✅.

catsby · 2020-06-23T11:01:18Z

Test failures are unrelated so I'm going to merge now. Thanks @calvn, @alexanderbez , and @pcman312 for the reviews 😄

* Resolve merge conflicts and updates from running a test * move testing/_test.go over to legacy * updates * Add core of plugin test framework Stepwise (#9166) * adding stepwise testing, but there are protocol buff error :/ * move file and update sdk/go.mo * update/sync modules * update from other branch * update sdk/go.mod * some cleanups after feedback * remove enviornments from this PR * update vendor * change from running go mod tidy * change from go mod tidy * Update sdk/testing/stepwise/helpers.go Co-authored-by: Michael Golowka <72365+pcman312@users.noreply.github.com> * Update sdk/testing/stepwise/helpers.go Co-authored-by: Michael Golowka <72365+pcman312@users.noreply.github.com> * change panic to error * Update sdk/testing/stepwise/helpers.go return `nil` and not `err` at the end Co-authored-by: Michael Golowka <72365+pcman312@users.noreply.github.com> * Defer close() on successful Open of a file * document the re-creation of steps * Update sdk/testing/stepwise/stepwise.go Co-authored-by: Michael Golowka <72365+pcman312@users.noreply.github.com> * remove unused BarrierKeys() * Update sdk/testing/stepwise/stepwise.go Co-authored-by: Michael Golowka <72365+pcman312@users.noreply.github.com> * updates from feedback * fix return with bad arguments * Rename things: - StepOperation -> Operation - StepwiseEnvironment -> Environment - StepCheckFunc -> AssertionFunc - step.Check -> step.Assert * document the environment interface methods * rename EnvironmentOptions to MountOptions * rename Name to RegistryName * remove ExpectError because it's redundant * minor doc update * Update sdk/testing/stepwise/stepwise.go Co-authored-by: Michael Golowka <72365+pcman312@users.noreply.github.com> * add checkShouldRun function * remove redundant return * remove vestigial PreCheck function * add tt.Helper() to makeRequest * minor code formatting and document 1-based index for log output of Steps Co-authored-by: Michael Golowka <72365+pcman312@users.noreply.github.com> * minor updates * update sdk * use local reference for api, vault dep * Update sdk/testing/stepwise/stepwise.go Co-authored-by: Calvin Leung Huang <cleung2010@gmail.com> * Update sdk/testing/stepwise/stepwise.go Co-authored-by: Calvin Leung Huang <cleung2010@gmail.com> * cleanup some defer functions * call fatal if environment setup fails, and don't call teardown * defer re-setting client token in makeRequest * Move legacy logicaltest back to testhelpers * update mods and test files with go mod tidy * go mod vendor * remove relative replace directives * restore old logical test location * move declaration to main stepwise file * remove index var and use i+1 * add testing for write, delete paths of makeRequest * update stepwise core testing to do request counting * remove unused methods * Update sdk/testing/stepwise/stepwise.go remove dead line Co-authored-by: Alexander Bezobchuk <alexanderbez@users.noreply.github.com> * Update sdk/testing/stepwise/stepwise.go fix capitalization in code comment Co-authored-by: Alexander Bezobchuk <alexanderbez@users.noreply.github.com> * update code comments for SkipTeardown to clarify its use * update stepwise Co-authored-by: Michael Golowka <72365+pcman312@users.noreply.github.com> Co-authored-by: Calvin Leung Huang <cleung2010@gmail.com> Co-authored-by: Alexander Bezobchuk <alexanderbez@users.noreply.github.com>

…nts/observations (hashicorp#9270) (hashicorp#9278) Co-authored-by: Violet Hynes <violet.hynes@hashicorp.com>

catsby and others added 11 commits June 3, 2020 17:12

Resolve merge conflicts and updates from running a test

a7fcbff

move testing/_test.go over to legacy

3208079

updates

191ddda

Merge branch 'master' into vlt091-plugin-testing

bb13341

* master: changelog++ certutil/helpers.go: Allow 3072 RSA key sizes. (#8343)

minor updates

9c2a34d

update sdk

fdb864a

use local reference for api, vault dep

f612cec

calvn reviewed Jun 19, 2020

View reviewed changes

Comment thread sdk/go.mod Outdated

catsby and others added 11 commits June 19, 2020 14:26

Update sdk/testing/stepwise/stepwise.go

5a95e20

Co-authored-by: Calvin Leung Huang <cleung2010@gmail.com>

Update sdk/testing/stepwise/stepwise.go

c35d76d

Co-authored-by: Calvin Leung Huang <cleung2010@gmail.com>

cleanup some defer functions

14557d1

call fatal if environment setup fails, and don't call teardown

483bc30

defer re-setting client token in makeRequest

09b2307

Move legacy logicaltest back to testhelpers

8027800

update mods and test files with go mod tidy

3e1f119

go mod vendor

827ec6f

remove relative replace directives

85134ad

restore old logical test location

1562a3f

Merge branch 'master' into vlt091-plugin-testing

db86ef5

* master: Fix to failing test compilation. (#9272) Added gauges to count KV secrets. (#9250)

catsby marked this pull request as ready for review June 19, 2020 21:07

catsby added this to the 1.5 milestone Jun 19, 2020

move declaration to main stepwise file

2765357

catsby requested a review from calvn June 19, 2020 22:08

calvn reviewed Jun 19, 2020

View reviewed changes

Comment thread sdk/go.mod

calvn reviewed Jun 19, 2020

View reviewed changes

Comment thread sdk/testing/stepwise/stepwise_test.go Outdated

catsby added 2 commits June 22, 2020 08:10

remove index var and use i+1

0737373

add testing for write, delete paths of makeRequest

2821e53

alexanderbez reviewed Jun 22, 2020

View reviewed changes

Comment thread sdk/go.mod

Comment thread sdk/testing/stepwise/stepwise.go

Comment thread sdk/testing/stepwise/stepwise.go Outdated

Comment thread sdk/testing/stepwise/stepwise.go Outdated

catsby and others added 5 commits June 22, 2020 15:03

update stepwise core testing to do request counting

8d5e1e6

remove unused methods

1bb36b0

Update sdk/testing/stepwise/stepwise.go

2faedc5

remove dead line Co-authored-by: Alexander Bezobchuk <alexanderbez@users.noreply.github.com>

Update sdk/testing/stepwise/stepwise.go

150abc1

fix capitalization in code comment Co-authored-by: Alexander Bezobchuk <alexanderbez@users.noreply.github.com>

update code comments for SkipTeardown to clarify its use

4951eab

calvn approved these changes Jun 22, 2020

View reviewed changes

update stepwise

2c4914a

catsby merged commit e926896 into master Jun 23, 2020

pull Bot pushed a commit to NOUIY/vault that referenced this pull request Sep 11, 2025

VAULT-39368 fix version not being populated for some KV mounts in eve…

0fe7702

…nts/observations (hashicorp#9270) (hashicorp#9278) Co-authored-by: Violet Hynes <violet.hynes@hashicorp.com>

Conversation

catsby commented Jun 19, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

calvn left a comment

Choose a reason for hiding this comment

Uh oh!

calvn Jun 19, 2020

Choose a reason for hiding this comment

Uh oh!

catsby Jun 19, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

calvn Jun 19, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

calvn left a comment

Choose a reason for hiding this comment

Uh oh!

catsby commented Jun 23, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

catsby commented Jun 19, 2020 •

edited

Loading