Parameter set controller #101

sgettys · 2022-05-27T22:03:37Z

Added controller for parameter set resource

Signed-off-by: Steven Gettys <[email protected]>

Signed-off-by: Brian DeGeeter <[email protected]>

codecov · 2022-05-27T22:15:57Z

Codecov Report

Merging #101 (7712105) into main (337a3f9) will decrease coverage by 0.19%.
The diff coverage is 76.92%.

@@            Coverage Diff             @@
##             main     #101      +/-   ##
==========================================
- Coverage   78.27%   78.07%   -0.20%     
==========================================
  Files          10       12       +2     
  Lines         833      976     +143     
==========================================
+ Hits          652      762     +110     
- Misses        115      136      +21     
- Partials       66       78      +12

Flag	Coverage Δ
unit-tests	`78.07% <76.92%> (-0.20%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
api/v1/parameterset_types.go	`66.66% <66.66%> (ø)`
controllers/parameterset_controller.go	`77.61% <77.61%> (ø)`

Continue to review full report at Codecov.

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

carolynvs

This is looking good! I've mostly just flagged some tech debt that we are increasing and want to say that going forward I can't accept more PRs that don't also fix these problems.

Makefile

api/v1/parameterset_types_test.go

carolynvs · 2022-06-02T13:46:42Z

controllers/parameterset_controller.go

+//+kubebuilder:rbac:groups=porter.sh,resources=parametersets,verbs=get;list;watch;create;update;patch;delete
+//+kubebuilder:rbac:groups=porter.sh,resources=parametersets/status,verbs=get;update;patch
+//+kubebuilder:rbac:groups=porter.sh,resources=parametersets/finalizers,verbs=update
+//+kubebuilder:rbac:groups=porter.sh,resources=agentconfigs,verbs=get;list;watch;create;update;patch;delete


Heads up that I don't think we need to repeat rbac comments from other controllers. I am pretty sure that you could remove everything here but the new rbac lines for parametersets and it wouldn't change any of the generated fields.

Here's the open issue to track cleaning this up but in the meantime we don't need to keep adding new code that has the same problem. 😀 #100

Will address with next PR that cleans up all this tech debt!

controllers/parameterset_controller.go

controllers/parameterset_controller_test.go

docs/content/file-formats.md

carolynvs · 2022-06-02T14:13:21Z

tests/integration/paramset_test.go

+		},
+		Spec: porterv1.ParameterSetSpec{
+			//TODO: get schema version from porter version
+			// https://github.com/getporter/porter/pull/2052


Heads up that this PR is merged, if you'd like to do a follow up PR to address the TODO in these tests. We could bump the version of Porter that we are referencing at the same time.

carolynvs · 2022-06-02T14:18:54Z

tests/integration/paramset_test.go

+	}
+}
+
+func waitForParamSetDeleted(ctx context.Context, ps *porterv1.ParameterSet) error {


The amount of copy/paste that is in these test files is really starting to concern me. We are just piling up on code that we know we don't want to maintain like this.

I've created #102 to tack consolidating these test helpers so that 80% of the test files aren't duplicate test helpers.

Next on the list, will be starting immediately

Signed-off-by: Steven Gettys <[email protected]>

carolynvs

Just two small fixes and we are good to go 👍

carolynvs · 2022-06-03T14:08:13Z

docs/content/file-formats.md

+The ParameterSet spec is the same schema as the ParameterSet resource in Porter.
+You can copy/paste the output of the `porter parameters show NAME -o yaml` command into the ParameterSet resource spec (removing the status section).
+
+In addition to the normal fields available on a [Porter Parameter Set document](/reference/file-formats/), the following fields are supported:


Can you update the link to go directly to the correct section on the page?

Whoops copy paste error :)

carolynvs · 2022-06-03T14:08:17Z

docs/content/file-formats.md

+The CredentialSet spec is the same schema as the CredentialSet resource in Porter.
+You can copy/paste the output of the `porter credentials show NAME -o yaml` command into the CredentialSet resource spec (removing the status section).
+
+In addition to the normal fields available on a [Porter Credential Set document](/reference/file-formats/), the following fields are supported:


Can you update the link to go directly to the correct section on the page?

Signed-off-by: Steven Gettys <[email protected]>

carolynvs

This is great, thank you for implementing parameter and credential sets! 🙇‍♀️

Steven Gettys and others added 4 commits May 27, 2022 15:02

chore: scaffolding for parameterset crd

1e93c54

Signed-off-by: Steven Gettys <[email protected]>

basic paramset reconciler with existing tests passing

0abe748

Signed-off-by: Brian DeGeeter <[email protected]>

initial passing paramset integration test

b5b9fca

Signed-off-by: Brian DeGeeter <[email protected]>

add paramset installation test

9d1f40f

Signed-off-by: Brian DeGeeter <[email protected]>

sgettys requested a review from carolynvs May 27, 2022 22:03

carolynvs self-assigned this Jun 2, 2022

carolynvs requested changes Jun 2, 2022

View reviewed changes

chore: Updated docs and added some test coverage

5a42613

Signed-off-by: Steven Gettys <[email protected]>

sgettys force-pushed the parameter_set_controller branch from 7a944f5 to 5a42613 Compare June 2, 2022 22:51

chore: Added parameter for helm chart version configuration

f9d4d7b

Signed-off-by: Steven Gettys <[email protected]>

carolynvs approved these changes Jun 3, 2022

View reviewed changes

docs: Fixed docs references

7712105

Signed-off-by: Steven Gettys <[email protected]>

carolynvs approved these changes Jun 3, 2022

View reviewed changes

carolynvs merged commit 38f717f into getporter:main Jun 3, 2022

Parameter set controller #101

Parameter set controller #101

Uh oh!

Conversation

sgettys commented May 27, 2022

Uh oh!

codecov bot commented May 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

carolynvs left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

carolynvs left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

carolynvs left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov bot commented May 27, 2022 •

edited

Loading