Align SDK with Kubebuilder - Project version stable #4402

camilamacedo86 · 2021-01-18T16:37:22Z

Description of the change:

Update kb dependency from v2.3.2-0.20201214213149-0a807f4e9428 to v3.0.0-alpha.0.0.20210203175028-3c8e370d49c5. See: kubernetes-sigs/kubebuilder@0a807f4...3c8e370 and fix modules to point out v3.
Upgrade gcr.io/kubebuilder/kube-rbac-proxy image to 0.5.0 to 0.8.0. (fix security concern)
Get Project version stable

Motivation for the change:

Partial: upgrade sdk to use kube-rbac-proxy:v0.7.0 instead of kube-rbac-proxy:v0.5.0 #3925
Closes: make install fails following Quickstart for Go-based Operators #4453
Solve tech debts and allow upstream and downstream still be compatible and maintainable.
Allow SDK still getting bug fixes and improvements done in upstream
Solve security concerns
Avoid breaking changes in the future that impacts users and maintainers.

Checklist

If the pull request includes user-facing changes, extra documentation is required:

Add a new changelog fragment in changelog/fragments (see changelog/fragments/00-template.yaml)
Add or update relevant sections of the docs website in website/content/en/docs

internal/plugins/ansible/v1/scaffolds/internal/templates/makefile.go

estroz

Personally I think the kubebuilder dep should not be upgraded until v3.0.0 is released and project version 3-alpha is stable, in case other changes are made to the project file spec. That way the migration is guaranteed to be a one time thing.

testdata/go/v2/memcached-operator/PROJECT

Adirio

First pass

Makefile

hack/generate/samples/generate_testdata.go

internal/cmd/operator-sdk/cli/cli.go

internal/cmd/operator-sdk/generate/internal/genutil.go

internal/plugins/helm/v1/scaffolds/internal/templates/makefile.go

internal/plugins/manifests/v2/plugin.go

internal/util/projutil/project_util.go

testdata/ansible/memcached-operator/Makefile

testdata/go/v2/memcached-operator/PROJECT

estroz

Still quite a bit to fix, and a few changes can be broken out into separate PRs since they aren't dependent on kubebuilder:

kube-rbac-proxy bump for ansible and helm
Makefile updates

hack/generate/samples/internal/go/v3/memcached_with_webhooks.go

internal/cmd/operator-sdk/generate/internal/genutil.go

internal/plugins/ansible/v1/api.go

internal/plugins/ansible/v1/scaffolds/api.go

internal/plugins/ansible/v1/scaffolds/internal/templates/makefile.go

internal/plugins/golang/v2/config.go

internal/plugins/helm/v1/api.go

internal/plugins/helm/v1/init.go

Adirio · 2021-02-09T22:48:55Z

After discussing with the parties in this PR, I'm good with this and having a hook to migrate v3-alpha to v3.

Just to clarify, the migration hook he is talking about is to migrate the project configuration (PROJECT file) version from 3-alpha to "3".

jmrodri · 2021-02-10T03:10:57Z

/lgtm

jmrodri · 2021-02-10T03:11:02Z

/retest

estroz

A bunch of nits and a few questions. Looks good after non-blocking changes are made

internal/plugins/helm/v1/api.go

internal/plugins/helm/v1/chartutil/chart.go

internal/plugins/helm/v1/chartutil/chart_test.go

estroz · 2021-02-10T19:00:02Z

internal/plugins/helm/v1/scaffolds/api.go

-	s.config.UpdateResources(res.GVK())
+	resource := resourceOptions.NewResource(s.config)
+
+	resource.Domain = s.config.GetDomain()


@Adirio is there a reason this isn't being set by NewResource()?

https://github.com/kubernetes-sigs/kubebuilder/blob/72f01e58c677219d7af38385a1b249b496391167/pkg/plugins/golang/options.go#L126-L139

It sets the Domain in the GVK of the Options. Previously, when configs are injected to subcommands, options.Domain = c.GetDomain() is called. This is, the easliest we have the config we set it in the Options and that gets transfered to the Resource.

Ah then this is redundant. @camilamacedo86 the scaffold templates should use Resource.GVK.Domain.

Just a small comment: Resource.Domain can also be accessed directly as the GVK is actually embedded (and inlined for json)

internal/plugins/manifests/v2/plugin.go

estroz · 2021-02-10T19:12:38Z

testdata/ansible/memcached-operator/PROJECT

@@ -1,11 +1,15 @@
 domain: example.com
 layout: ansible.sdk.operatorframework.io/v1
+plugins:


Hmm this field was deliberately marshaled last so it would always be at the bottom. There might be a regression upstream @Adirio.

It was marshalled separately to do some config/v2 logic and with the config.Config interface with different implementations for each version that logic is no longer needed so it is now being marshalled at the same time that the rest. I can't find any reason why we should keep this last, but if you have any we can handle this in two steps again.

Mainly because that key contains multiple other configs that could grow very large down the line. Its a cosmetic thing. I guess I'm fine leaving it as-is, but that was the original intent of marshaling it last.

Okey I see your point. Luckyly, loading doesn't care about the order and each time you call a plugin the file is re-written, so if we decide to go back to keeping plugins as the last key, it will be backwards compatible with every project. That means that we can tackle this issue when we find it.

internal/testutils/olm.go

internal/util/projutil/project_util.go

internal/plugins/ansible/v1/scaffolds/api.go

Signed-off-by: Camila Macedo <cmacedo@redhat.com>

estroz

@camilamacedo86 nice work!

/lgtm

Update the kube-rbac-proxy image to use v0.8.0, which runs as nonroot. This change was done in operator-sdk v1.5.0, but was not noted in the migration guide. See operator-framework/operator-sdk#4402

openshift-ci-robot requested review from estroz and jmccormick2001 January 18, 2021 16:37

camilamacedo86 force-pushed the fix-project-bump-kb branch from 7eaff31 to 9fe1d52 Compare January 18, 2021 17:18

estroz reviewed Jan 18, 2021

View reviewed changes

internal/plugins/ansible/v1/scaffolds/internal/templates/makefile.go Outdated Show resolved Hide resolved

estroz suggested changes Jan 18, 2021

View reviewed changes

camilamacedo86 force-pushed the fix-project-bump-kb branch 3 times, most recently from 60f0af8 to de03ee2 Compare January 21, 2021 15:40

This comment has been minimized.

Sign in to view

camilamacedo86 requested a review from estroz January 21, 2021 15:48

This comment has been minimized.

Sign in to view

camilamacedo86 force-pushed the fix-project-bump-kb branch 3 times, most recently from 3372450 to 9fe9be7 Compare January 21, 2021 20:00

camilamacedo86 requested a review from jmrodri January 21, 2021 20:03

This comment has been minimized.

Sign in to view

camilamacedo86 force-pushed the fix-project-bump-kb branch from 9fe9be7 to 80812e4 Compare January 22, 2021 12:48

camilamacedo86 mentioned this pull request Jan 29, 2021

make install fails following Quickstart for Go-based Operators #4453

Closed

camilamacedo86 force-pushed the fix-project-bump-kb branch 2 times, most recently from ccf4ebe to de17d18 Compare January 29, 2021 22:07

camilamacedo86 changed the title ~~update kb dep and get bugfixes~~ WIP: update kb dep and get bugfixes Jan 30, 2021

openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 30, 2021

camilamacedo86 commented Jan 30, 2021

View reviewed changes

testdata/go/v2/memcached-operator/PROJECT Show resolved Hide resolved

Adirio suggested changes Feb 1, 2021

View reviewed changes

estroz suggested changes Feb 1, 2021

View reviewed changes

Adirio mentioned this pull request Feb 1, 2021

✨ Return a typed error in case DecodePluginConfig was unable to find the provided key kubernetes-sigs/kubebuilder#1985

Merged

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 3, 2021

camilamacedo86 force-pushed the fix-project-bump-kb branch 2 times, most recently from a214328 to 20be9fd Compare February 3, 2021 13:13

camilamacedo86 force-pushed the fix-project-bump-kb branch from e627b4d to f415d00 Compare February 10, 2021 01:41

openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Feb 10, 2021

camilamacedo86 force-pushed the fix-project-bump-kb branch from f415d00 to 0aa775a Compare February 10, 2021 02:15

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 10, 2021

estroz suggested changes Feb 10, 2021

View reviewed changes

camilamacedo86 force-pushed the fix-project-bump-kb branch from 0aa775a to 2ab33a6 Compare February 10, 2021 20:23

openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Feb 10, 2021

camilamacedo86 requested a review from estroz February 10, 2021 20:24

camilamacedo86 closed this Feb 10, 2021

camilamacedo86 reopened this Feb 10, 2021

estroz reviewed Feb 10, 2021

View reviewed changes

internal/plugins/ansible/v1/scaffolds/api.go Show resolved Hide resolved

camilamacedo86 force-pushed the fix-project-bump-kb branch from 2ab33a6 to 5ab84a1 Compare February 11, 2021 16:06

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 11, 2021

camilamacedo86 force-pushed the fix-project-bump-kb branch from 5ab84a1 to 13b77ce Compare February 11, 2021 16:10

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 11, 2021

camilamacedo86 requested a review from estroz February 11, 2021 16:12

align the sdk with kb

2f1e522

Signed-off-by: Camila Macedo <cmacedo@redhat.com>

camilamacedo86 force-pushed the fix-project-bump-kb branch from 13b77ce to 2f1e522 Compare February 11, 2021 16:41

estroz approved these changes Feb 11, 2021

View reviewed changes

openshift-ci-robot assigned estroz Feb 11, 2021

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 11, 2021

jmrodri merged commit 4fc8a17 into operator-framework:master Feb 11, 2021

joelanford mentioned this pull request Feb 24, 2021

Update for kubebuilder v3 operator-framework/helm-operator-plugins#87

Closed

slopezz mentioned this pull request Mar 19, 2021

Ansible operator-sdk v1.5.0 with updated kube-rbac-proxy:v0.8.0 fails to run with permission denied #4684

Closed

adambkaplan mentioned this pull request Dec 1, 2021

Use kube-rbac-proxy v0.8.0 shipwright-io/operator#37

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Align SDK with Kubebuilder - Project version stable #4402

Align SDK with Kubebuilder - Project version stable #4402

camilamacedo86 commented Jan 18, 2021 •

edited

Loading

estroz left a comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Adirio left a comment

estroz left a comment

Adirio commented Feb 9, 2021

jmrodri commented Feb 10, 2021

jmrodri commented Feb 10, 2021

estroz left a comment

estroz Feb 10, 2021

Adirio Feb 10, 2021

estroz Feb 10, 2021

Adirio Feb 10, 2021

estroz Feb 10, 2021

Adirio Feb 10, 2021

estroz Feb 10, 2021

Adirio Feb 10, 2021

estroz left a comment

Align SDK with Kubebuilder - Project version stable #4402

Align SDK with Kubebuilder - Project version stable #4402

Conversation

camilamacedo86 commented Jan 18, 2021 • edited Loading

estroz left a comment

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Adirio left a comment

Choose a reason for hiding this comment

estroz left a comment

Choose a reason for hiding this comment

Adirio commented Feb 9, 2021

jmrodri commented Feb 10, 2021

jmrodri commented Feb 10, 2021

estroz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

estroz left a comment

Choose a reason for hiding this comment

camilamacedo86 commented Jan 18, 2021 •

edited

Loading