operator-framework / operator-sdk

SDK for building Kubernetes applications. Provides high level APIs, useful abstractions, and project scaffolding.
https://sdk.operatorframework.io
Apache License 2.0
7.24k stars 1.74k forks source link

Go CSV Markers not included in CSV when types come from multiple go packages #6624

Closed Nicolas-Duboc-IBM closed 6 months ago

Nicolas-Duboc-IBM commented 11 months ago

Bug Report

What did you do?

In a Go operator project, spread the types of the API in multiple go packages:

A sample project is available at https://github.com/Nicolas-Duboc-IBM/operatorsdk-multipackage-sample

File operatorsdk-multipackage-sample/api/v1alpha1/example_types.go :

package v1alpha1

import (
    "github.com/Nicolas-Duboc-IBM/multi-package-api-operator/api/common"
    metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

//+kubebuilder:object:root=true
//+kubebuilder:subresource:status
// Example is the Schema for the examples API
type Example struct {
    metav1.TypeMeta   `json:",inline"`
    metav1.ObjectMeta `json:"metadata,omitempty"`

    Spec   common.ExampleSpec `json:"spec,omitempty"`
    Status ExampleStatus      `json:"status,omitempty"`
}
....

File operatorsdk-multipackage-sample/api/common/common.go :

package common

// ExampleSpec defines the desired state of Example
type ExampleSpec struct {
    // Foo is an example field of Example. Edit example_types.go to remove/update
    Foo string `json:"foo,omitempty"`

    // Bar is another exemple field
    // +operator-sdk:csv:customresourcedefinitions:type=spec,xDescriptors={"urn:alm:descriptor:com.tectonic.ui:hidden"}
    Bar string `json:"bar,omitempty"`

    Options CommonOptions `json:"options"`
}

// CommonOptions defines some common options of our apis
type CommonOptions struct {
    // Common1 is an option
    Common1 string `json:"common1,omitempty"`

    // Hidden1 is an internal options that should be hidden in UI
    // +operator-sdk:csv:customresourcedefinitions:type=spec,xDescriptors={"urn:alm:descriptor:com.tectonic.ui:hidden"}
    Hidden1 string `json:"hidden1,omitempty"`
}

What did you expect to see?

After running operator-sdk generate kustomize manifests, or more typically make bundle, I expected to find an entry for path: options.hidden1 with the urn:alm:descriptor:com.tectonic.ui:hidden property in the generated CSV in file config/manifests/bases/operator-sdk-defect.clusterserviceversion.yaml.

What did you see instead? Under which circumstances?

Only the fields of the top-level types are included in the CSV. The fields of lower-level types are not considered.

File config/manifests/bases/operator-sdk-defect.clusterserviceversion.yaml :

  customresourcedefinitions:
    owned:
    - description: Example is the Schema for the examples API
      displayName: Example
      kind: Example
      name: examples.cache.example.com
      specDescriptors:
      - description: Bar is another exemple field
        displayName: Bar
        path: bar
        x-descriptors:
        - urn:alm:descriptor:com.tectonic.ui:hidden
      version: v1alpha1

There should be an entry for options.hidden1 :

      - description: Hidden1 is an internal options that should be hidden in UI
        displayName: Hidden1
        path: options.hidden1
        x-descriptors:
        - urn:alm:descriptor:com.tectonic.ui:hidden

Environment

Operator type:

/language go

$ operator-sdk version
operator-sdk version: "v1.32.0-6-g034aef67", commit: "034aef6779729a30ac3d716f93a01824ed8dd3c7", kubernetes version: "v1.26.0", go version: "go1.19.1", GOOS: "darwin", GOARCH: "arm64"
$ go version
go version go1.19.1 darwin/arm64

Possible Solution

Additional context

Nicolas-Duboc-IBM commented 11 months ago

Changes from PR #6626 are a proposal to fix this issue.

varshaprasad96 commented 11 months ago

@Nicolas-Duboc-IBM Could you elaborate on what is the reason for having a nested spec defined in a separate package? Wondering on what is the necessity to have a different package and not have it in one.

Nicolas-Duboc-IBM commented 11 months ago

Starting from an existing operator managing a single Kind, we added a second Kind with a very similar structure and same management code. During this refactoring we moved all the shared types into a "common" package and the top-level types in their own packages also. After second thought, I realize that we could have kept all the types in the same package and simply splitted into multiple go files. Actually I just tested moving back all the types in a single package and it works also. operator-sdk then generates the correct CSV with all the expected markers. Then perhaps this problem could be fixed simply with more documentation about this limitation.

Nicolas-Duboc-IBM commented 11 months ago

Actually it seems that multiple go packages are required to manage kinds in different API group. Because the +groupName annotation is put on go packages.

varshaprasad96 commented 11 months ago

@Nicolas-Duboc-IBM that's right, this is the expected behaviour with markers in controller-tools. This is because the marker on the parent has no way to discover the fields in the child struct. Would you be open to adding documentation on this?

openshift-bot commented 8 months ago

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale. Stale issues rot after an additional 30d of inactivity and eventually close. Exclude this issue from closing by commenting /lifecycle frozen.

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

/lifecycle stale

openshift-bot commented 7 months ago

Stale issues rot after 30d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle rotten. Rotten issues close after an additional 30d of inactivity. Exclude this issue from closing by commenting /lifecycle frozen.

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

/lifecycle rotten /remove-lifecycle stale

openshift-bot commented 6 months ago

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen. Mark the issue as fresh by commenting /remove-lifecycle rotten. Exclude this issue from closing again by commenting /lifecycle frozen.

/close

openshift-ci[bot] commented 6 months ago

@openshift-bot: Closing this issue.

In response to [this](https://github.com/operator-framework/operator-sdk/issues/6624#issuecomment-2080407906): >Rotten issues close after 30d of inactivity. > >Reopen the issue by commenting `/reopen`. >Mark the issue as fresh by commenting `/remove-lifecycle rotten`. >Exclude this issue from closing again by commenting `/lifecycle frozen`. > >/close Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.