Skip to content

feat: add kb scaffold utilities for plugins in internal/kubebuilder#3305

Merged
camilamacedo86 merged 3 commits intooperator-framework:masterfrom
camilamacedo86:add-internal-kb
Jul 7, 2020
Merged

feat: add kb scaffold utilities for plugins in internal/kubebuilder#3305
camilamacedo86 merged 3 commits intooperator-framework:masterfrom
camilamacedo86:add-internal-kb

Conversation

@camilamacedo86
Copy link
Copy Markdown
Contributor

@camilamacedo86 camilamacedo86 commented Jun 27, 2020

Description

  • Just add the KB internal in internal/kubebuilder
  • change hack/check-license.sh to allow kb license

Motivation
SDK is in a process to be integrated with KB which means that its project layouts will be aligned. More info : Integrating Kubebuilder and Operator SDK. As the first step, for we getting starting to working on in the Ansible/Helm plugins with the new layout we defined the strategy to just copy these helpers which will allow us to have the same features/utilities required to do the scaffolds.

Note that these utilities are not exported by kubebuilder, in this way, since Ansible/Helm will be also plugins that will do similar scaffolds and following a similar design they will be useful in the next steps. For a further understanding see the WIP: #329 and the doc Extensible CLI and Scaffolding Plugins.

@camilamacedo86 camilamacedo86 added the kubebuilder-integration Relates to rewriting the SDK in Kubebuilder plugin form label Jun 27, 2020
@joelanford
Copy link
Copy Markdown
Member

I know we discussed this offline, but could you expand on the reasoning for this in the PR description for those that might wonder why we're duplicating KB code.

@camilamacedo86
Copy link
Copy Markdown
Contributor Author

@joelanford it is done.

@camilamacedo86 camilamacedo86 changed the title add kb internal in internal/kubebuilder feat: add kb scaffold utilities for plugins in internal/kubebuilder Jun 30, 2020
Comment thread internal/kubebuilder/cmdutil/cmdutil.go Outdated
Copy link
Copy Markdown
Member

@joelanford joelanford left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good except copy the files verbatim (no changes to the license headers)

@camilamacedo86
Copy link
Copy Markdown
Contributor Author

All good except copy the files verbatim (no changes to the license headers)
Done 👍

@camilamacedo86 camilamacedo86 added the language/helm Issue is related to a Helm operator project label Jul 1, 2020
Comment thread internal/kubebuilder/validation/dns.go Outdated
Comment on lines +24 to +26
// This file's code was modified from "k8s.io/apimachinery/pkg/util/validation"
// to avoid package dependencies. In case of additional functionality from
// "k8s.io/apimachinery" is needed, re-consider whether to add the dependency.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since operator-sdk already depends on k8s.io/apimachinery, can we remove this file?

It looks like regexError (used in internal/kubebuilder/validation/project.go) is now exported in k8s.io/apimachinery, so we could make that one tweak to that file.

Copy link
Copy Markdown
Contributor Author

@camilamacedo86 camilamacedo86 Jul 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can remove the full validation dir. Done. See that it is already removed from #3295

Copy link
Copy Markdown
Member

@joelanford joelanford left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 7, 2020
@camilamacedo86 camilamacedo86 merged commit c4df8d2 into operator-framework:master Jul 7, 2020
@camilamacedo86 camilamacedo86 deleted the add-internal-kb branch July 7, 2020 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kubebuilder-integration Relates to rewriting the SDK in Kubebuilder plugin form language/helm Issue is related to a Helm operator project lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants