📖 : add faq section on 3rd party APIs in envtest

[varshaprasad96]

This commit adds a section on how to use third party APIs
in envtest.

Signed-off-by: varshaprasad96 [email protected]

Addresses - #1191

[coderanger]

Looks good, if we want to go even further maybe an example?

[estroz]

/lgtm

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: estroz, varshaprasad96
To complete the pull request process, please assign vincepri after the PR has been reviewed.
You can assign the PR to them by writing /assign @vincepri in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[k8s-ci-robot]

New changes are detected. LGTM label has been removed.

[varshaprasad96]

Since this involves adding the api to scheme (which is explained in the previous question), and just specifying the path in CRDDirectoryPaths, is this example section required ? @coderanger @camilamacedo86

[camilamacedo86]

Hi @varshaprasad96 and @estroz,

Really thank you for work on this. It is a widespread scenario that brings a lot of confusion, and then, I believe that we need to try to cover it in a better way.

Could EnvTest check/use the schema(s) on the cluster instead of expecting us to inform the CRDs indeed for the external types? Would not that possible? Should not it indeed that a better solution? If we reach the consensus that yes, then I think we can add only in the FAQ and clarify it is a workaround until the issue #nnnn be addressed. @vincepri @DirectXMan12 wdyt?

Please, check the Why section below.

Suggested Solution (only we cannot improve envtest / or until we have a better solution in place)

• Tell the user that currently, EnvTest requires to have access to all schemas definitions (CRDs) that will be used by it and then, because of this, the user needs to provide the CRD of the external type.
• Recommend first with the example of the go mod option, which is better than downloading the CRD imo:

CRDDirectoryPaths: []string{
			filepath.Join("..", "config", "crd", "bases"),
			filepath.Join(build.Default.GOPATH, "pkg", "mod", "github.com", "owner", "theirs@version", "config", "crd", "bases"),
},

• Then, describe that also is possible to add external type CRD schema(s) by adding the manifest on the project
• Recommend to the user to create a new directory and not suggested it or lead them to believe that they should download it in the config/crd/bases. PS.: IMO the best place in this scenario would be where the suite_test is (e.g controllers/testdata/external)
• Tell to them add the downloaded CRD in the new directory. See that CRDDirectoryPaths is a list of paths so, the user can inform the new directory as well:

               CRDDirectoryPaths: []string{
			filepath.Join("..", "config", "crd", "bases"),
                        filepath.Join("testdata", "external"), 
		},

Why

The config/ has the project config manifests. So, I do not think that we should recommend or provide any info that leads to the idea that people should download and add the external CRDs there which are ONLY useful for the tests. It can cause harmful side effects since the tools (sdk/kb) expected found in the config/crd/bases ONLY the CRDs which are defined in the project e.g:

• after adding the CRD the users will run make install and try to apply on cluster a schema that should exist already there
• SDK will use all CRDs to generate the bundle and will add the external-type to it. @estroz do we want to have the external-type CRD defined in the csv and copied to bundle/manifests/. Will OLM not try to install them on the cluster? Will that work if we have operator A which is the ower installing the CRD via the bundle, and then, operator B which is the consumer trying to install the same CRD again in the same cluster? I do not think so.

PS.: @pwittrock's initial idea about allows exploding files from config-gen would be only for that one which makes sense for users customize, ditto so it would not work with this recommendation and could lead a misunderstands.

Note that we can provide helpers in SDK or KB or any other consumer/tool that makes the user's life easier. However, we cannot suggest a recommendation that can lead to an idea that might hurt the project's config or that is not exactly a good practice such as add things that are only useful for specific tests in the default/global configuration of the project. I think we need to be careful with what we suggest as a solution. Otherwise, the helpers will not work well as expected.

[mpreu]

(I am sorry if this is not the right place for my objection)

In the current discussions about this topic (operator-sdk and here) there are basically two promoted solutions (at least as a workaround until better support):

1. Downloading the external CRDs
2. Referring to a go module containing the external CRDs

1.) has potential downsides as already mentioned, while 2.) introduces the problem of having a statically defined path floating around, which needs to be updated manually if dependencies in the project change. 2.) can also lead to unexpected errors if multiple versions are available in the go module cache and someone forgets to update the tests to refer to the new version of external CRDs.

As external CRDs should usually be a dependency of the project anyway (as testdata or within the controller), could it be possible to promote using go modules in the regular way and dynamically add the CRDs to envtest using the go.mod file?

E.g. adding OCP CRDs programmatically could look (very) roughly like this:

func getOCPCRDs() []string {
	const ocpModule = "github.com/openshift/api"
	modFilePath := filepath.Join("..", "..", "go.mod")

	goModBytes, err := ioutil.ReadFile(modFilePath)
	Expect(err).NotTo(HaveOccurred())

	file, err := modfile.Parse(modFilePath, goModBytes, nil)
	Expect(err).NotTo(HaveOccurred())

	ocpVersion := ""
	for _, r := range file.Require {
		if r.Mod.Path == ocpModule {
			ocpVersion = r.Mod.Version
		}
	}
	Expect(ocpVersion).ToNot(BeEmpty())

	basePath := filepath.Join(
		build.Default.GOPATH,
		"pkg",
		"mod",
		ocpModule+"@"+ocpVersion,
	)

	netNamespaceCRDPath := filepath.Join(
		basePath,
		"network",
		"v1",
		"003-netnamespace-crd.yaml",
	)

	networkConfigCRDPath := filepath.Join(
		basePath,
		"config",
		"v1",
		"0000_10_config-operator_01_network.crd.yaml",
	)

	return []string{
		netNamespaceCRDPath,
		networkConfigCRDPath,
	}
}
...
By("extracting relevant OpenShift API CRDs")
ocpCRDs := getOCPCRDs()

By("bootstrapping test environment")
testEnv = &envtest.Environment{
    ErrorIfCRDPathMissing: true,
    CRDDirectoryPaths: []string{
        filepath.Join("..", "..", "config", "crd", "bases"),
    },
}
testEnv.CRDDirectoryPaths = append(testEnv.CRDDirectoryPaths, ocpCRDs...)

Outlined in this simple way it has obvious problems as well, but at least removes the need to update the static go module path in the tests with every little dependency update in the overall project.

[k8s-triage-robot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale

[k8s-triage-robot]

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

[k8s-triage-robot]

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Reopen this issue or PR with /reopen
• Mark this issue or PR as fresh with /remove-lifecycle rotten
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

[k8s-ci-robot]

@k8s-triage-robot: Closed this PR.

In response to this:

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Reopen this issue or PR with /reopen
• Mark this issue or PR as fresh with /remove-lifecycle rotten
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.