From 36ec0656580b2fa4d1134b388bd2bb44ab4eabcf Mon Sep 17 00:00:00 2001 From: Eric Stroczynski Date: Mon, 6 Apr 2020 14:31:49 -0700 Subject: [PATCH 1/4] use controller-tools/pkg/markers to parse CSV annotations in types hack/generate/migrate-markers.sh: annotation -> marker migration script internal/generate/olm-catalog: use legacy descriptor parsing if neither PROJECT nor the new markers format exists --- hack/generate/migrate-markers.sh | 76 +++++ .../bases/clusterserviceversion.go | 44 ++- .../bases/definitions/ast.go | 105 ++++++ .../bases/definitions/crd.go | 207 ++++++++++++ .../bases/definitions/definitions.go | 208 ++++++++++++ .../bases/definitions/definitions_test.go | 251 ++++++++++++++ .../bases/definitions/parse.go | 241 ++++++++++++++ .../bases/definitions/parse_test.go | 96 ++++++ .../definitions/zz_generated.markerhelp.go | 67 ++++ .../clusterserviceversion/bases/markers.go | 48 ++- internal/generate/crd/crd_test.go | 20 +- ...ith-ui-metadata.clusterserviceversion.yaml | 2 - ...cached-operator.clusterserviceversion.yaml | 2 - .../generate/testdata/go/api/v1alpha2/doc.go | 18 + .../testdata/go/api/v1alpha2/dummy_types.go | 173 ++++++++++ .../go/api/v1alpha2/memcached_types.go | 57 ++++ .../go/api/v1alpha2/memcachedrs_types.go | 55 +++ .../api/cache/v1alpha1/memcached_types.go | 6 +- ...operator.v0.0.1.clusterserviceversion.yaml | 2 - ...operator.v0.0.3.clusterserviceversion.yaml | 2 - ...operator.v0.0.4.clusterserviceversion.yaml | 2 - internal/markers/markers.go | 19 ++ .../golang/references/markers.md | 313 ++++++++---------- 23 files changed, 1799 insertions(+), 215 deletions(-) create mode 100755 hack/generate/migrate-markers.sh create mode 100644 internal/generate/clusterserviceversion/bases/definitions/ast.go create mode 100644 internal/generate/clusterserviceversion/bases/definitions/crd.go create mode 100644 internal/generate/clusterserviceversion/bases/definitions/definitions.go create mode 100644 internal/generate/clusterserviceversion/bases/definitions/definitions_test.go create mode 100644 internal/generate/clusterserviceversion/bases/definitions/parse.go create mode 100644 internal/generate/clusterserviceversion/bases/definitions/parse_test.go create mode 100644 internal/generate/clusterserviceversion/bases/definitions/zz_generated.markerhelp.go create mode 100644 internal/generate/testdata/go/api/v1alpha2/doc.go create mode 100644 internal/generate/testdata/go/api/v1alpha2/dummy_types.go create mode 100644 internal/generate/testdata/go/api/v1alpha2/memcached_types.go create mode 100644 internal/generate/testdata/go/api/v1alpha2/memcachedrs_types.go create mode 100644 internal/markers/markers.go diff --git a/hack/generate/migrate-markers.sh b/hack/generate/migrate-markers.sh new file mode 100755 index 0000000000..fb408c5c1d --- /dev/null +++ b/hack/generate/migrate-markers.sh @@ -0,0 +1,76 @@ +#!/usr/bin/env bash + +# Migrate old CSV annotations to new markers. +# Example: +# $ ./migrate-markers.sh *.go + +OLD_MARKER_BASE="operator-sdk:gen-csv:customresourcedefinitions" +NEW_MARKER_BASE="operator-sdk:csv:customresourcedefinitions" + +SPEC_TYPE="spec" +STATUS_TYPE="status" + +function migrate_displayName() { + local old_marker_pattern="${OLD_MARKER_BASE}"'\.displayName="([^"]+)"' + local new_marker_pattern="${NEW_MARKER_BASE}"':displayName="\1"' + + sed -i -E 's/'$old_marker_pattern'/'$new_marker_pattern'/g' "$1" +} + +function migrate_resources() { + local old_marker_pattern_1="${OLD_MARKER_BASE}"'\.resources="([^\,]+)\,([^\,]+)\,\\"([^\\]+)\\""' + local old_marker_pattern_2="${OLD_MARKER_BASE}"'\.resources="([^\,]+)\,([^\,]+)\,"' + local new_marker_pattern_1="${NEW_MARKER_BASE}"':resources={\1,\2,"\3"}' + local new_marker_pattern_2="${NEW_MARKER_BASE}"':resources={\1,\2,}' + + sed -i -E 's/'$old_marker_pattern_1'/'$new_marker_pattern_1'/g' "$1" + sed -i -E 's/'$old_marker_pattern_2'/'$new_marker_pattern_2'/g' "$1" +} + +function migrate_typeDescriptors() { + local type=$2 + local old_marker_true_pattern="${OLD_MARKER_BASE}\.${type}Descriptors=true" + local old_marker_false_pattern="${OLD_MARKER_BASE}\.${type}Descriptors=false" + local new_marker_pattern="${NEW_MARKER_BASE}:type=${type}" + + sed -i -E 's/'$old_marker_true_pattern'/'$new_marker_pattern'/g' "$1" + sed -i -E 's/'$old_marker_false_pattern'//g' "$1" +} + +function migrate_typeDescriptors_displayName() { + local type=$2 + local old_marker_pattern="${OLD_MARKER_BASE}\.${type}Descriptors"'\.displayName="([^"]+)"' + local new_marker_pattern="${NEW_MARKER_BASE}:type=${type}"',displayName="\1"' + + sed -i -E 's/'$old_marker_pattern'/'$new_marker_pattern'/g' "$1" +} + +function migrate_typeDescriptors_xDescriptors() { + local type=$2 + local old_marker_pattern="${OLD_MARKER_BASE}\.${type}Descriptors"'\.x-descriptors="([^"]+)"' + local new_marker_pattern="${NEW_MARKER_BASE}:type=${type}"',xDescriptors="\1"' + + sed -i -E 's/'$old_marker_pattern'/'$new_marker_pattern'/g' "$1" +} + +set -eu +shopt -s extglob nullglob +FILES=$* +shopt -u extglob nullglob + +for file in $FILES; do + if ! [[ "$file" =~ .*\.go ]]; then + continue + fi + # Globals + migrate_displayName "$file" + migrate_resources "$file" + # Spec descriptors + migrate_typeDescriptors "$file" $SPEC_TYPE + migrate_typeDescriptors_displayName "$file" $SPEC_TYPE + migrate_typeDescriptors_xDescriptors "$file" $SPEC_TYPE + # Status descriptors + migrate_typeDescriptors "$file" $STATUS_TYPE + migrate_typeDescriptors_displayName "$file" $STATUS_TYPE + migrate_typeDescriptors_xDescriptors "$file" $STATUS_TYPE +done diff --git a/internal/generate/clusterserviceversion/bases/clusterserviceversion.go b/internal/generate/clusterserviceversion/bases/clusterserviceversion.go index 970ca7528a..d9a8415513 100644 --- a/internal/generate/clusterserviceversion/bases/clusterserviceversion.go +++ b/internal/generate/clusterserviceversion/bases/clusterserviceversion.go @@ -15,9 +15,13 @@ package bases import ( + "bufio" "bytes" "fmt" "io/ioutil" + "os" + "path/filepath" + "strings" "github.com/operator-framework/api/pkg/operators/v1alpha1" log "github.com/sirupsen/logrus" @@ -25,7 +29,9 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "sigs.k8s.io/yaml" + "github.com/operator-framework/operator-sdk/internal/markers" "github.com/operator-framework/operator-sdk/internal/util/k8sutil" + kbutil "github.com/operator-framework/operator-sdk/internal/util/kubebuilder" "github.com/operator-framework/operator-sdk/internal/util/projutil" ) @@ -80,7 +86,14 @@ func (b ClusterServiceVersion) GetBase() (base *v1alpha1.ClusterServiceVersion, if b.APIsDir != "" { switch b.OperatorType { case projutil.OperatorTypeGo: - if err := updateDescriptionsForGVKs(base, b.APIsDir, b.GVKs); err != nil { + // Update descriptions from the APIs dir. + // TODO(estroz): remove this condition once old annotations are deprecated. + if kbutil.HasProjectFile() || areAnnotationsMigrated(b.APIsDir) { + err = updateDefinitions(base, b.APIsDir, b.GVKs) + } else { + err = updateDescriptionsForGVKs(base, b.APIsDir, b.GVKs) + } + if err != nil { return nil, fmt.Errorf("error generating ClusterServiceVersion base metadata: %w", err) } } @@ -197,3 +210,32 @@ func readClusterServiceVersionBase(path string) (*v1alpha1.ClusterServiceVersion return nil, fmt.Errorf("no ClusterServiceVersion manifest in %s", path) } + +// areAnnotationsMigrated returns true if annotations have been migrated to the new markers format. +// Errors are ignored so the caller can default to legacy behavior. +func areAnnotationsMigrated(apisRoot string) bool { + if apisRoot == "" { + return false + } + hasMarkers := false + _ = filepath.Walk(apisRoot, func(path string, info os.FileInfo, err error) error { + if err != nil || info.IsDir() || filepath.Ext(path) != ".go" { + return err + } + f, err := os.Open(path) + if err != nil { + return err + } + scanner := bufio.NewScanner(f) + for scanner.Scan() { + // The new prefix "operator-sdk:csv:" differs from the old one, "operator-sdk:gen-csv:". + if strings.Contains(scanner.Text(), markers.Prefix+":csv:") { + hasMarkers = true + break + } + } + _ = f.Close() + return scanner.Err() + }) + return hasMarkers +} diff --git a/internal/generate/clusterserviceversion/bases/definitions/ast.go b/internal/generate/clusterserviceversion/bases/definitions/ast.go new file mode 100644 index 0000000000..cd4418b975 --- /dev/null +++ b/internal/generate/clusterserviceversion/bases/definitions/ast.go @@ -0,0 +1,105 @@ +// Copyright 2020 The Operator-SDK Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package definitions + +import ( + "errors" + "fmt" + "go/ast" + "strings" + + "sigs.k8s.io/controller-tools/pkg/markers" +) + +// getMarkedChildrenOfField collects all marked fields from type delcarations starting at root in depth-first order. +func (g generator) getMarkedChildrenOfField(root markers.FieldInfo) (map[string][]*fieldInfo, error) { + // ast.Inspect will not traverse into fields, so iteratively collect them and to check for markers. + nextFields := []*fieldInfo{{FieldInfo: root}} + markedFields := map[string][]*fieldInfo{} + for len(nextFields) > 0 { + fields := []*fieldInfo{} + for _, field := range nextFields { + errs := []error{} + ast.Inspect(field.RawField, func(n ast.Node) bool { + if n == nil { + return true + } + switch expr := n.(type) { + case *ast.Ident: + // Only look at type names. + if expr.Obj == nil || expr.Obj.Kind != ast.Typ { + return true + } + // Check if the field's type exists in the known types. + info, hasInfo := g.types[expr.Name] + if !hasInfo { + return true + } + // Add all child fields to the list to search next. + for _, finfo := range info.Fields { + segment, err := getPathSegmentForField(finfo) + if err != nil { + errs = append(errs, fmt.Errorf("error getting path from type %s field %s: %v", + info.Name, finfo.Name, err), + ) + return true + } + // Add extra information to the segment if it comes from a certain field type. + switch finfo.RawField.Type.(type) { + case (*ast.ArrayType): + // arrayFieldGroup case. + if segment != ignoredTag && segment != inlinedTag { + segment += "[0]" + } + } + // Create a new set of path segments using the parent's segments + // and add the field to the next fields to search. + f := &fieldInfo{ + FieldInfo: finfo, + pathSegments: append(field.pathSegments, segment), + } + fields = append(fields, f) + // Marked fields get collected for the caller to parse. + if len(finfo.Markers) != 0 { + markedFields[info.Name] = append(markedFields[info.Name], f) + } + } + } + return true + }) + if err := fmtParseErrors(errs); err != nil { + return nil, err + } + } + nextFields = fields + } + return markedFields, nil +} + +// fmtParseErrors prettifies a list of errors to make them easier to read. +func fmtParseErrors(errs []error) error { + switch len(errs) { + case 0: + return nil + case 1: + return errs[0] + } + sb := strings.Builder{} + for _, err := range errs { + sb.WriteString("\n") + sb.WriteString(err.Error()) + } + return errors.New(sb.String()) +} diff --git a/internal/generate/clusterserviceversion/bases/definitions/crd.go b/internal/generate/clusterserviceversion/bases/definitions/crd.go new file mode 100644 index 0000000000..67bd8ae798 --- /dev/null +++ b/internal/generate/clusterserviceversion/bases/definitions/crd.go @@ -0,0 +1,207 @@ +// Copyright 2020 The Operator-SDK Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package definitions + +import ( + "fmt" + "reflect" + "sort" + "strings" + + "github.com/fatih/structtag" + "github.com/operator-framework/api/pkg/operators/v1alpha1" + "github.com/operator-framework/operator-registry/pkg/registry" + "k8s.io/apimachinery/pkg/version" + "sigs.k8s.io/controller-tools/pkg/markers" + + "github.com/operator-framework/operator-sdk/internal/util/k8sutil" +) + +// MakeKeyForCRDDescription creates a key unique to a crdDescription. +func MakeKeyForCRDDescription(crd v1alpha1.CRDDescription) registry.DefinitionKey { + return registry.DefinitionKey{ + Name: crd.Name, + Group: MakeFullGroupForName(crd.Name), + Version: crd.Version, + Kind: crd.Kind, + } +} + +// MakeCRDDescriptionForKey creates a crdDescription from a unique key. +func MakeCRDDescriptionForKey(key registry.DefinitionKey) v1alpha1.CRDDescription { + return v1alpha1.CRDDescription{ + Name: key.Name, + Version: key.Version, + Kind: key.Kind, + DisplayName: k8sutil.GetDisplayName(key.Kind), + } +} + +// MakeFullGroupForName returns everything but the first element of a CRD name, +// which by definition is .. +func MakeFullGroupForName(name string) string { + return getHalfBySep(name, ".", 1) +} + +// MakeGroupForFullGroup returns the first element of an API group, ex. "foo" of "foo.example.com". +func MakeGroupForFullGroup(group string) string { + return getHalfBySep(group, ".", 0) +} + +// getHalfBySep splits s by the first sep encountered and returns the first +// (half = 0) or second (half = 1) element of the result. +func getHalfBySep(s, sep string, half uint) string { + if split := strings.SplitN(s, sep, 2); len(split) == 2 && half < 2 { + return split[half] + } + return s +} + +// buildCRDDescriptionFromType builds a crdDescription for the Go API defined +// by key from markers and type information in g.types. +func (g generator) buildCRDDescriptionFromType(key registry.DefinitionKey, + kindType *markers.TypeInfo) (v1alpha1.CRDDescription, error) { + + // Initialize the description. + description := MakeCRDDescriptionForKey(key) + description.Description = kindType.Doc + + // Parse resources and displayName from the kind type's markers. + for _, markers := range kindType.Markers { + for _, marker := range markers { + if d, isDescription := marker.(Description); isDescription { + if d.DisplayName != "" { + description.DisplayName = d.DisplayName + } + if len(d.Resources) != 0 { + refs, err := d.Resources.toResourceReferences() + if err != nil { + return v1alpha1.CRDDescription{}, err + } + description.Resources = append(description.Resources, refs...) + } + } + } + } + sortDescription(description.Resources) + + // Find spec and status in the kind type. + spec, err := findChildForDescType(kindType, specDescType) + if err != nil { + return v1alpha1.CRDDescription{}, err + } + status, err := findChildForDescType(kindType, statusDescType) + if err != nil { + return v1alpha1.CRDDescription{}, err + } + + // Find annotated fields of spec and parse them into specDescriptors. + markedFields, err := g.getMarkedChildrenOfField(spec) + if err != nil { + return v1alpha1.CRDDescription{}, err + } + specDescriptors := []v1alpha1.SpecDescriptor{} + for _, fields := range markedFields { + for _, field := range fields { + if descriptor, include := field.toSpecDescriptor(); include { + specDescriptors = append(specDescriptors, descriptor) + } + } + } + sortDescriptors(specDescriptors) + description.SpecDescriptors = specDescriptors + + // Find annotated fields of status and parse them into statusDescriptors. + markedFields, err = g.getMarkedChildrenOfField(status) + if err != nil { + return v1alpha1.CRDDescription{}, err + } + statusDescriptors := []v1alpha1.StatusDescriptor{} + for _, fields := range markedFields { + for _, field := range fields { + if descriptor, include := field.toStatusDescriptor(); include { + statusDescriptors = append(statusDescriptors, descriptor) + } + } + } + sortDescriptors(statusDescriptors) + description.StatusDescriptors = statusDescriptors + + return description, nil +} + +// findChildForDescType returns a field with a tag matching string(typ) by searching all top-level fields in info. +// If no field is found, an error is returned. +func findChildForDescType(info *markers.TypeInfo, typ descType) (markers.FieldInfo, error) { + for _, field := range info.Fields { + tags, err := structtag.Parse(string(field.Tag)) + if err != nil { + return markers.FieldInfo{}, err + } + jsonTag, err := tags.Get("json") + if err == nil && jsonTag.Name == string(typ) { + return field, nil + } + } + return markers.FieldInfo{}, fmt.Errorf("no %s found for type %s", typ, info.Name) +} + +// sortDescriptors sorts a slice of structs with a Path field by comparing Path strings naturally. +func sortDescriptors(v interface{}) { + slice := reflect.ValueOf(v) + values := toValueSlice(slice) + sort.Slice(values, func(i, j int) bool { + return values[i].FieldByName("Path").String() < values[j].FieldByName("Path").String() + }) + for i := 0; i < slice.Len(); i++ { + slice.Index(i).Set(values[i]) + } +} + +// sortDescription sorts a slice of structs with Name, Kind, and Version fields +// by comparing those field's strings in natural order. +func sortDescription(v interface{}) { + slice := reflect.ValueOf(v) + values := toValueSlice(slice) + sort.Slice(values, func(i, j int) bool { + nameI := values[i].FieldByName("Name").String() + nameJ := values[j].FieldByName("Name").String() + if nameI == nameJ { + kindI := values[i].FieldByName("Kind").String() + kindJ := values[j].FieldByName("Kind").String() + if kindI == kindJ { + versionI := values[i].FieldByName("Version").String() + versionJ := values[j].FieldByName("Version").String() + return version.CompareKubeAwareVersionStrings(versionI, versionJ) > 0 + } + return kindI < kindJ + } + return nameI < nameJ + }) + for i := 0; i < slice.Len(); i++ { + slice.Index(i).Set(values[i]) + } +} + +// toValueSlice creates a slice of values that can be sorted by arbitrary fields. +func toValueSlice(slice reflect.Value) []reflect.Value { + sliceCopy := reflect.MakeSlice(slice.Type(), slice.Len(), slice.Len()) + reflect.Copy(sliceCopy, slice) + values := make([]reflect.Value, sliceCopy.Len()) + for i := 0; i < sliceCopy.Len(); i++ { + values[i] = sliceCopy.Index(i) + } + return values +} diff --git a/internal/generate/clusterserviceversion/bases/definitions/definitions.go b/internal/generate/clusterserviceversion/bases/definitions/definitions.go new file mode 100644 index 0000000000..0366556689 --- /dev/null +++ b/internal/generate/clusterserviceversion/bases/definitions/definitions.go @@ -0,0 +1,208 @@ +// Copyright 2020 The Operator-SDK Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package definitions + +import ( + "errors" + "fmt" + "os" + "path/filepath" + + "github.com/operator-framework/api/pkg/operators/v1alpha1" + "github.com/operator-framework/operator-registry/pkg/registry" + log "github.com/sirupsen/logrus" + "golang.org/x/tools/go/packages" + "sigs.k8s.io/controller-tools/pkg/genall" + "sigs.k8s.io/controller-tools/pkg/loader" + "sigs.k8s.io/controller-tools/pkg/markers" +) + +type descriptionValues struct { + crd v1alpha1.CRDDescription + // TODO(estroz): support apiServiceDescriptions +} + +// ApplyDefinitionsForKeysGo collects markers and AST info on Go type declarations and struct fields +// to populate csv spec fields. Go code with relevant markers and information is expected to be +// in a package under apisRootDir and match a GVK in keys. +func ApplyDefinitionsForKeysGo(csv *v1alpha1.ClusterServiceVersion, apisRootDir string, + keys []registry.DefinitionKey) error { + + // Construct a set of probable paths under apisRootDir for types defined by keys. + // These are usually '(pkg/)?apis/(/)?'. + // NB(estroz): using "leaf" packages prevents type builders from searching other packages. + // It would be nice to implement extra-package traversal in the future. + paths, err := makeAPIPaths(apisRootDir, keys) + if err != nil { + return err + } + // Some APIs may not exist under apisRootDir, so skip loading packages if no paths are found + if len(paths) == 0 { + return nil + } + + // Collect Go types from roots. + g := &generator{} + ctx, err := g.contextForRoots(paths...) + if err != nil { + return err + } + g.needTypes(ctx) + if loader.PrintErrors(ctx.Roots, packages.TypeError) { + return errors.New("one or more API packages had type errors") + } + + // Create definitions for kind types found under the collected roots. + definitionsByKey := make(map[registry.DefinitionKey]*descriptionValues) + fmt.Println("building types for:", keys) + for _, key := range keys { + kindType, hasKind := g.types[key.Kind] + if !hasKind { + log.Debugf("Skipping CSV annotation parsing for API %s: type %s not found", key, key.Kind) + continue + } + crd, err := g.buildCRDDescriptionFromType(key, kindType) + if err != nil { + return err + } + definitionsByKey[key] = &descriptionValues{ + crd: crd, + } + } + + // Update csv with all values parsed. + updateDefinitionsByKey(csv, definitionsByKey) + + return nil +} + +// makeAPIPaths creates a set of API directory paths with apisRootDir as their parent. +func makeAPIPaths(apisRootDir string, keys []registry.DefinitionKey) (paths []string, err error) { + if apisRootDir, err = filepath.Abs(apisRootDir); err != nil { + return nil, err + } + + for _, key := range keys { + // Check if the kind pkg is at the expected layout. + group := MakeGroupForFullGroup(key.Group) + expectedPkgPath, err := getExpectedPkgLayout(apisRootDir, group, key.Version) + if err != nil { + return nil, err + } + if expectedPkgPath == "" { + log.Debugf("Skipping CSV annotation parsing for API %s: directory does not exist", key) + continue + } + paths = append(paths, expectedPkgPath) + } + return paths, nil +} + +// updateDefinitionsByKey updates owned definitions that already exist in csv or adds new definitions that do not. +func updateDefinitionsByKey(csv *v1alpha1.ClusterServiceVersion, + definitionsByKey map[registry.DefinitionKey]*descriptionValues) { + + // Overwrite crdDescriptions we've parsed from Go source. + for i := 0; i < len(csv.Spec.CustomResourceDefinitions.Owned); i++ { + crd := csv.Spec.CustomResourceDefinitions.Owned[i] + key := MakeKeyForCRDDescription(crd) + if values, hasKey := definitionsByKey[key]; hasKey { + csv.Spec.CustomResourceDefinitions.Owned[i] = values.crd + delete(definitionsByKey, key) + } + } + + // Add any new crdDescriptions to the CSV. + for _, values := range definitionsByKey { + csv.Spec.CustomResourceDefinitions.Owned = append(csv.Spec.CustomResourceDefinitions.Owned, values.crd) + } +} + +func isDirExist(path string) (bool, error) { + fileInfo, err := os.Stat(path) + if err != nil { + if os.IsNotExist(err) { + return false, nil + } + return false, err + } + return fileInfo.IsDir(), nil +} + +// getExpectedPkgLayout checks the directory layout in apisRootDir for single and multi group layouts and returns +// the expected pkg path for the group and version. Returns empty string if neither single or multi group layout +// is detected. +// - multi-group layout: apis// +// - single-group layout: api/ +func getExpectedPkgLayout(apisRootDir, group, version string) (expectedPkgPath string, err error) { + if group == "" || version == "" { + return "", nil + } + groupVersionDir := filepath.Join(apisRootDir, group, version) + if isMultiGroupLayout, err := isDirExist(groupVersionDir); isMultiGroupLayout { + if err != nil { + return "", err + } + return groupVersionDir, nil + } + versionDir := filepath.Join(apisRootDir, version) + if isSingleGroupLayout, err := isDirExist(versionDir); isSingleGroupLayout { + if err != nil { + return "", err + } + return versionDir, nil + } + // Neither multi nor single group layout + return "", nil +} + +// generator creates API definitions from type information for a set of roots. +type generator struct { + types map[string]*markers.TypeInfo +} + +// contextForRoots creates a context that can populate a generator for a set of roots loaded from dirs. +// These roots contain data for registered markers. +func (g *generator) contextForRoots(dirs ...string) (ctx *genall.GenerationContext, err error) { + roots, err := loader.LoadRoots(dirs...) + if err != nil { + return ctx, err + } + registry := &markers.Registry{} + if err := registerMarkers(registry); err != nil { + return ctx, err + } + return &genall.GenerationContext{ + Collector: &markers.Collector{ + Registry: registry, + }, + Roots: roots, + InputRule: genall.InputFromFileSystem, + Checker: &loader.TypeChecker{}, + }, nil +} + +// needTypes sets types in the generator for a given context. +func (g *generator) needTypes(ctx *genall.GenerationContext) { + g.types = make(map[string]*markers.TypeInfo) + cb := func(info *markers.TypeInfo) { + g.types[info.Name] = info + } + for _, root := range ctx.Roots { + if err := markers.EachType(ctx.Collector, root, cb); err != nil { + root.AddError(err) + } + } +} diff --git a/internal/generate/clusterserviceversion/bases/definitions/definitions_test.go b/internal/generate/clusterserviceversion/bases/definitions/definitions_test.go new file mode 100644 index 0000000000..af3dd2a933 --- /dev/null +++ b/internal/generate/clusterserviceversion/bases/definitions/definitions_test.go @@ -0,0 +1,251 @@ +// Copyright 2020 The Operator-SDK Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package definitions + +import ( + "os" + "path/filepath" + "testing" + + "github.com/operator-framework/api/pkg/operators/v1alpha1" + "github.com/operator-framework/operator-registry/pkg/registry" + "github.com/stretchr/testify/assert" +) + +var ( + testDataDir = filepath.Join("..", "..", "..", "testdata", "go") +) + +func TestApplyDefinitionsForKeysGo(t *testing.T) { + + wd, err := os.Getwd() + if err != nil { + t.Fatal(err) + } + if err := os.Chdir(testDataDir); err != nil { + t.Fatal(err) + } + defer func() { + if err = os.Chdir(wd); err != nil { + t.Fatal(err) + } + }() + + //nolint:dupl + cases := []struct { + description string + apisDir string + csv *v1alpha1.ClusterServiceVersion + keys []registry.DefinitionKey + expectedCRDs v1alpha1.CustomResourceDefinitions + wantErr bool + }{ + { + description: "Populate CRDDescription successfully", + apisDir: "api", + csv: &v1alpha1.ClusterServiceVersion{}, + keys: []registry.DefinitionKey{ + {Name: "dummies.cache.example.com", Group: "cache.example.com", Version: "v1alpha2", Kind: "Dummy"}, + }, + expectedCRDs: v1alpha1.CustomResourceDefinitions{ + Owned: []v1alpha1.CRDDescription{ + { + Name: "dummies.cache.example.com", + Kind: "Dummy", + Version: "v1alpha2", + DisplayName: "Dummy App", + Description: "Dummy is the Schema for the dummy API", + Resources: []v1alpha1.APIResourceReference{ + {Name: "dummy-deployment", Kind: "Deployment", Version: "v1"}, + {Name: "dummy-pod", Kind: "Pod", Version: "v1"}, + {Name: "dummy-replicaset", Kind: "ReplicaSet", Version: "v1beta2"}, + }, + SpecDescriptors: []v1alpha1.SpecDescriptor{ + {Path: "size", DisplayName: "dummy-size", Description: "Should be in spec", + XDescriptors: []string{"urn:alm:descriptor:com.tectonic.ui:podCount"}}, + {Path: "wheels", DisplayName: "Wheels", + Description: "Should be in spec, but should not have array index in path", + XDescriptors: []string{"urn:alm:descriptor:com.tectonic.ui:text"}}, + {Path: "wheels[0].type", DisplayName: "Wheel Type", + Description: "Type should be in spec with path equal to wheels[0].type", + XDescriptors: []string{ + "urn:alm:descriptor:com.tectonic.ui:arrayFieldGroup:wheels", + "urn:alm:descriptor:com.tectonic.ui:text", + }}, + }, + StatusDescriptors: []v1alpha1.StatusDescriptor{ + {Path: "hog.engine", DisplayName: "boss-hog-engine", XDescriptors: []string{}, + Description: "Should be in status but not spec, since Hog isn't in DummySpec"}, + {Path: "hog.foo", DisplayName: "Public", XDescriptors: []string{}}, + {Path: "hog.seatMaterial", DisplayName: "Seat Material", XDescriptors: []string{}}, + {Path: "hog.seatMaterial", DisplayName: "Seat Material", XDescriptors: []string{}}, + {Path: "nodes", DisplayName: "Nodes", XDescriptors: []string{}, + Description: "Should be in status but not spec, since DummyStatus isn't in DummySpec"}, + }, + }, + }, + }, + }, + { + description: "Populate CRDDescription with non-standard spec type successfully", + apisDir: "api", + csv: &v1alpha1.ClusterServiceVersion{}, + keys: []registry.DefinitionKey{ + {Name: "otherdummies.cache.example.com", Group: "cache.example.com", Version: "v1alpha2", Kind: "OtherDummy"}, + }, + expectedCRDs: v1alpha1.CustomResourceDefinitions{ + Owned: []v1alpha1.CRDDescription{ + { + Name: "otherdummies.cache.example.com", + Kind: "OtherDummy", + Version: "v1alpha2", + DisplayName: "Other Dummy App", + Description: "OtherDummy is the Schema for the other dummy API", + Resources: []v1alpha1.APIResourceReference{ + {Name: "other-dummy-pod", Kind: "Pod", Version: "v1"}, + {Name: "other-dummy-service", Kind: "Service", Version: "v1"}, + }, + SpecDescriptors: []v1alpha1.SpecDescriptor{ + {Path: "engine", DisplayName: "Engine", XDescriptors: []string{}, + Description: "Should be in status but not spec, since Hog isn't in DummySpec"}, + {Path: "foo", DisplayName: "Public", XDescriptors: []string{}}, + {Path: "seatMaterial", DisplayName: "Seat Material", XDescriptors: []string{}}, + {Path: "seatMaterial", DisplayName: "Seat Material", XDescriptors: []string{}}, + }, + StatusDescriptors: []v1alpha1.StatusDescriptor{ + {Path: "nothing", DisplayName: "Nothing", XDescriptors: []string{}, + Description: "Should be in status but not spec, since this isn't a spec type"}, + }, + }, + }, + }, + }, + { + description: "Do not change definitions with non-existent package dir", + apisDir: filepath.Join("pkg", "notexist"), + csv: &v1alpha1.ClusterServiceVersion{ + Spec: v1alpha1.ClusterServiceVersionSpec{ + CustomResourceDefinitions: v1alpha1.CustomResourceDefinitions{ + Owned: []v1alpha1.CRDDescription{ + { + Name: "dummies.cache.example.com", Version: "v1alpha2", Kind: "Dummy", + DisplayName: "Dummy App", + Description: "Dummy is the Schema for the other dummy API", + Resources: []v1alpha1.APIResourceReference{ + {Name: "dummy-pod", Kind: "Pod", Version: "v1"}, + }, + SpecDescriptors: []v1alpha1.SpecDescriptor{ + {Path: "foo", DisplayName: "Foo", XDescriptors: []string{}, + Description: "Should not be removed"}, + }, + StatusDescriptors: []v1alpha1.StatusDescriptor{ + {Path: "bar", DisplayName: "Bar", XDescriptors: []string{}, + Description: "Should not be removed"}, + }, + }, + }, + }, + }, + }, + keys: []registry.DefinitionKey{ + {Name: "dummies.cache.example.com", Group: "cache.example.com", Version: "v1alpha2", Kind: "Dummy"}, + }, + expectedCRDs: v1alpha1.CustomResourceDefinitions{ + Owned: []v1alpha1.CRDDescription{ + { + Name: "dummies.cache.example.com", Version: "v1alpha2", Kind: "Dummy", + DisplayName: "Dummy App", + Description: "Dummy is the Schema for the other dummy API", + Resources: []v1alpha1.APIResourceReference{ + {Name: "dummy-pod", Kind: "Pod", Version: "v1"}, + }, + SpecDescriptors: []v1alpha1.SpecDescriptor{ + {Path: "foo", DisplayName: "Foo", XDescriptors: []string{}, + Description: "Should not be removed"}, + }, + StatusDescriptors: []v1alpha1.StatusDescriptor{ + {Path: "bar", DisplayName: "Bar", XDescriptors: []string{}, + Description: "Should not be removed"}, + }, + }, + }, + }, + }, + { + description: "Do not change definitions with non-existent type", + apisDir: "api", + csv: &v1alpha1.ClusterServiceVersion{ + Spec: v1alpha1.ClusterServiceVersionSpec{ + CustomResourceDefinitions: v1alpha1.CustomResourceDefinitions{ + Owned: []v1alpha1.CRDDescription{ + { + Name: "nokinds.cache.example.com", Version: "v1alpha2", Kind: "NoKind", + DisplayName: "NoKind App", + Description: "NoKind is the Schema for the other nokind API", + Resources: []v1alpha1.APIResourceReference{ + {Name: "no-kind-pod", Kind: "Pod", Version: "v1"}, + }, + SpecDescriptors: []v1alpha1.SpecDescriptor{ + {Path: "foo", DisplayName: "Foo", XDescriptors: []string{}, + Description: "Should not be removed"}, + }, + StatusDescriptors: []v1alpha1.StatusDescriptor{ + {Path: "bar", DisplayName: "Bar", XDescriptors: []string{}, + Description: "Should not be removed"}, + }, + }, + }, + }, + }, + }, + keys: []registry.DefinitionKey{ + {Name: "nokinds.cache.example.com", Group: "cache.example.com", Version: "v1alpha2", Kind: "NoKind"}, + }, + expectedCRDs: v1alpha1.CustomResourceDefinitions{ + Owned: []v1alpha1.CRDDescription{ + { + Name: "nokinds.cache.example.com", Version: "v1alpha2", Kind: "NoKind", + DisplayName: "NoKind App", + Description: "NoKind is the Schema for the other nokind API", + Resources: []v1alpha1.APIResourceReference{ + {Name: "no-kind-pod", Kind: "Pod", Version: "v1"}, + }, + SpecDescriptors: []v1alpha1.SpecDescriptor{ + {Path: "foo", DisplayName: "Foo", XDescriptors: []string{}, + Description: "Should not be removed"}, + }, + StatusDescriptors: []v1alpha1.StatusDescriptor{ + {Path: "bar", DisplayName: "Bar", XDescriptors: []string{}, + Description: "Should not be removed"}, + }, + }, + }, + }, + }, + } + + for _, c := range cases { + t.Run(c.description, func(t *testing.T) { + err := ApplyDefinitionsForKeysGo(c.csv, c.apisDir, c.keys) + if !c.wantErr && err != nil { + t.Errorf("Expected nil error, got %q", err) + } else if c.wantErr && err == nil { + t.Errorf("Expected non-nil error, got nil error") + } else if !c.wantErr && err == nil { + assert.Equal(t, c.expectedCRDs, c.csv.Spec.CustomResourceDefinitions) + } + }) + } +} diff --git a/internal/generate/clusterserviceversion/bases/definitions/parse.go b/internal/generate/clusterserviceversion/bases/definitions/parse.go new file mode 100644 index 0000000000..9239203144 --- /dev/null +++ b/internal/generate/clusterserviceversion/bases/definitions/parse.go @@ -0,0 +1,241 @@ +// Copyright 2020 The Operator-SDK Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package definitions + +import ( + "fmt" + "reflect" + "strings" + "unicode" + + "github.com/fatih/structtag" + "github.com/operator-framework/api/pkg/operators/v1alpha1" + "sigs.k8s.io/controller-tools/pkg/markers" + + sdkmarkers "github.com/operator-framework/operator-sdk/internal/markers" + "github.com/operator-framework/operator-sdk/internal/util/k8sutil" +) + +const ( + // csvPrefix is the prefix for all csv marker names. + csvPrefix = sdkmarkers.Prefix + ":csv" + // crdMarkerName is the base marker name for all customresourcedefinitions markers. + crdMarkerName = csvPrefix + ":customresourcedefinitions" +) + +// +operator-sdk:csv:customresourcedefinitions:displayName="string",resources={ {kind,version,name} , ... } +var typeDefinition = markers.Must(markers.MakeDefinition(crdMarkerName, markers.DescribesType, Description{})) + +// +operator-sdk:csv:customresourcedefinitions:type=,displayName="name",xDescriptors="ui:elements:foo:bar" +var fieldDefinition = markers.Must(markers.MakeDefinition(crdMarkerName, markers.DescribesField, Descriptor{})) + +// registerMarkers adds type and field marker definitions to a registry. +func registerMarkers(into *markers.Registry) error { + if err := into.Register(typeDefinition); err != nil { + return fmt.Errorf("error registering type definition: %v", err) + } + into.AddHelp(typeDefinition, Description{}.Help()) + if err := into.Register(fieldDefinition); err != nil { + return fmt.Errorf("error registering field definition: %v", err) + } + into.AddHelp(fieldDefinition, Descriptor{}.Help()) + return nil +} + +// Resource is a list of strings defining a CRD description resource. +type Resource []string + +// Resources is a list of resource definitions. +type Resources []Resource + +// +controllertools:marker:generateHelp:category=Description + +// Description is a type used to receive type-level CRD description markers. +type Description struct { + // Resources is a list of string lists, each of which defines a CRD description resource. The marker format is: + // { { "kind" , "version" ( , "name")? } , ... } + Resources Resources `marker:",optional"` + // DisplayName is the displayName of a CRD description. + DisplayName string `marker:",optional"` +} + +// +controllertools:marker:generateHelp:category=Descriptor + +// Descriptor is a type used to receive field-level spec and status descriptor markers. Format of marker: +type Descriptor struct { + // Type is one of: "spec", "status". + Type string `marker:",optional"` + // DisplayName is the displayName of a spec or status description. + DisplayName string `marker:",optional"` + // XDescriptors is a list of UI path strings. The marker format is: + // "ui:element:foo,ui:element:bar" + XDescriptors []string `marker:",optional"` +} + +// toResourceReferences transforms Resources into a apiResourceReference slice. +func (resources Resources) toResourceReferences() (rs []v1alpha1.APIResourceReference, err error) { + for _, resource := range resources { + if l := len(resource); l < 2 { + return nil, fmt.Errorf("resource %+q did not have at least a kind and a version", resource) + } + r := v1alpha1.APIResourceReference{ + Kind: strings.TrimSpace(resource[0]), + Version: strings.TrimSpace(resource[1]), + } + if len(resource) == 3 { + r.Name = strings.TrimSpace(resource[2]) + } + rs = append(rs, r) + } + return rs, nil +} + +// fieldInfo is a markers.FieldInfo wrapper that also holds path segments. +type fieldInfo struct { + markers.FieldInfo + pathSegments []string +} + +// descType is a string identifying a descriptor type. +type descType string + +const ( + specDescType descType = "spec" + statusDescType descType = "status" +) + +// toStatusDescriptor transforms a fieldInfo into a specDescriptor. +func (fi fieldInfo) toSpecDescriptor() (descriptor v1alpha1.SpecDescriptor, include bool) { + include = fi.setDescriptorFields(&descriptor, specDescType) + return +} + +// toStatusDescriptor transforms a fieldInfo into a statusDescriptor. +func (fi fieldInfo) toStatusDescriptor() (descriptor v1alpha1.StatusDescriptor, include bool) { + include = fi.setDescriptorFields(&descriptor, statusDescType) + return +} + +// setDescriptorFields sets a struct with Description, Path, DisplayName, and XDescriptors fields by reflection. +func (fi fieldInfo) setDescriptorFields(d interface{}, typ descType) bool { + path, include := makePath(fi.pathSegments) + if !include { + return false + } + + seenDescType := false + displayName, xDescriptors := "", []string{} + for _, markers := range fi.Markers { + for _, marker := range markers { + d, isDescriptor := marker.(Descriptor) + if isDescriptor && d.Type == string(typ) { + if d.DisplayName != "" && displayName == "" { + displayName = d.DisplayName + } + xDescriptors = append(xDescriptors, d.XDescriptors...) + seenDescType = true + } + } + } + if displayName == "" { + displayName = k8sutil.GetDisplayName(fi.Name) + } + + v := reflect.ValueOf(d) + v.Elem().FieldByName("Description").SetString(fi.Doc) + v.Elem().FieldByName("Path").SetString(path) + v.Elem().FieldByName("DisplayName").SetString(displayName) + v.Elem().FieldByName("XDescriptors").Set(reflect.ValueOf(xDescriptors)) + + return seenDescType +} + +// makePath creates a path string from raw path segments. These segments can encode extra information +// about what field it came from. If a path should be ignored by the caller, it returns false. +func makePath(rawSegments []string) (string, bool) { + pathSegments := []string{} + for i, segment := range rawSegments { + switch { + case segment == ignoredTag: + // Ignored fields are not serialized and therefore its own path segment + // and those of its children should not be included in the path. + return "", false + case segment == inlinedTag: + // Inlined struct types move their fields into their parents, so the path segment + // of such a field should not be in the path if it is last in the path. + if i == len(rawSegments)-1 { + return "", false + } + continue + case strings.HasSuffix(segment, "[0]") && i == len(rawSegments)-1: + // Only include an arrayFieldGroup suffix if there is a child path segment. + segment = strings.TrimSuffix(segment, "[0]") + } + pathSegments = append(pathSegments, segment) + } + return strings.Join(pathSegments, "."), true +} + +const ( + inlinedTag = "##inline##" + ignoredTag = "##ignore##" +) + +// getPathSegmentForField parses a path segment from a field's tag. +func getPathSegmentForField(finfo markers.FieldInfo) (string, error) { + // Embedded fields are inlined and children may be included. + if len(finfo.RawField.Names) == 0 { + return inlinedTag, nil + } + // Unexported fields should be ignored in downstream processing. + if isNotExported(finfo.Name) { + return ignoredTag, nil + } + tags, err := structtag.Parse(string(finfo.Tag)) + if err != nil { + return "", err + } + jsonTag, err := tags.Get("json") + if err == nil { + // Parse returns an error if no JSON tag is in tags, at which point we'll use another method to get path. + switch { + case contains(jsonTag.Options, "inline"): + return inlinedTag, nil + case jsonTag.Name == "-": + if len(jsonTag.Options) == 0 { + return ignoredTag, nil + } + return jsonTag.Name, nil + case jsonTag.Name != "": + return jsonTag.Name, nil + } + } + // There is no JSON tag in tags or tag name is empty. Use info name as path as json.Marshal does. + return finfo.Name, nil +} + +// isNotExported returns true if name is not an exported struct field name. +func isNotExported(name string) bool { + return len(name) == 0 || unicode.IsLower(rune(name[0])) +} + +func contains(options []string, key string) bool { + for _, opt := range options { + if opt == key { + return true + } + } + return false +} diff --git a/internal/generate/clusterserviceversion/bases/definitions/parse_test.go b/internal/generate/clusterserviceversion/bases/definitions/parse_test.go new file mode 100644 index 0000000000..08a334aa86 --- /dev/null +++ b/internal/generate/clusterserviceversion/bases/definitions/parse_test.go @@ -0,0 +1,96 @@ +// Copyright 2020 The Operator-SDK Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package definitions + +import ( + "reflect" + "testing" + + "github.com/operator-framework/api/pkg/operators/v1alpha1" +) + +func TestParseResource(t *testing.T) { + cases := []struct { + description string + input Resources + exp []v1alpha1.APIResourceReference + wantErr bool + }{ + { + "Resource with no name", + Resources{{"Pod", "v1"}}, + []v1alpha1.APIResourceReference{{Kind: "Pod", Version: "v1"}}, + false, + }, + { + "Resource with name", + Resources{{"Pod", "v1", "memcached-pod"}}, + []v1alpha1.APIResourceReference{{Kind: "Pod", Version: "v1", Name: "memcached-pod"}}, + false, + }, + { + "Resource with string literal name", + Resources{{"Pod", "v1", `"memcached-pod"`}}, + []v1alpha1.APIResourceReference{{Kind: "Pod", Version: "v1", Name: `"memcached-pod"`}}, + false, + }, + { + "Two resources", + Resources{{"Pod", "v1", "memcached-pod"}, {"Service", "v1"}}, + []v1alpha1.APIResourceReference{ + {Kind: "Pod", Version: "v1", Name: "memcached-pod"}, + {Kind: "Service", Version: "v1"}, + }, + false, + }, + { + "Empty resource string without quotes", + Resources{{""}}, + []v1alpha1.APIResourceReference{}, + true, + }, + { + "Empty resource string with quotes", + Resources{{`""`}}, + []v1alpha1.APIResourceReference{}, + true, + }, + { + "Resource string with no version", + Resources{{"Memcached"}}, + []v1alpha1.APIResourceReference{}, + true, + }, + } + + for _, c := range cases { + output, err := c.input.toResourceReferences() + if err != nil { + if !c.wantErr { + t.Errorf("%s: expected nil error, got %q", c.description, err) + } + continue + } else if c.wantErr { + t.Errorf("%s: expected non-nil error, got nil error", c.description) + continue + } + + if !c.wantErr { + if !reflect.DeepEqual(c.exp, output) { + t.Errorf("%s: expected %s, got %s", c.description, c.exp, output) + } + } + } +} diff --git a/internal/generate/clusterserviceversion/bases/definitions/zz_generated.markerhelp.go b/internal/generate/clusterserviceversion/bases/definitions/zz_generated.markerhelp.go new file mode 100644 index 0000000000..15798e1c85 --- /dev/null +++ b/internal/generate/clusterserviceversion/bases/definitions/zz_generated.markerhelp.go @@ -0,0 +1,67 @@ +// +build !ignore_autogenerated + +// Copyright 2020 The Operator-SDK Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Code generated by helpgen. DO NOT EDIT. + +package definitions + +import ( + "sigs.k8s.io/controller-tools/pkg/markers" +) + +func (Description) Help() *markers.DefinitionHelp { + return &markers.DefinitionHelp{ + Category: "Description", + DetailedHelp: markers.DetailedHelp{ + Summary: "is a type used to receive type-level CRD description markers.", + Details: "", + }, + FieldHelp: map[string]markers.DetailedHelp{ + "Resources": markers.DetailedHelp{ + Summary: "is a list of string lists, each of which defines a CRD description resource. The marker format is: { { \"kind\" , \"version\" ( , \"name\")? } , ... }", + Details: "", + }, + "DisplayName": markers.DetailedHelp{ + Summary: "is the displayName of a CRD description.", + Details: "", + }, + }, + } +} + +func (Descriptor) Help() *markers.DefinitionHelp { + return &markers.DefinitionHelp{ + Category: "Descriptor", + DetailedHelp: markers.DetailedHelp{ + Summary: "is a type used to receive field-level spec and status descriptor markers. Format of marker:", + Details: "", + }, + FieldHelp: map[string]markers.DetailedHelp{ + "Type": markers.DetailedHelp{ + Summary: "is one of: \"spec\", \"status\".", + Details: "", + }, + "DisplayName": markers.DetailedHelp{ + Summary: "is the displayName of a spec or status description.", + Details: "", + }, + "XDescriptors": markers.DetailedHelp{ + Summary: "is a list of UI path strings. The marker format is: \"ui:element:foo,ui:element:bar\"", + Details: "", + }, + }, + } +} diff --git a/internal/generate/clusterserviceversion/bases/markers.go b/internal/generate/clusterserviceversion/bases/markers.go index cfe3a6fc56..9565525124 100644 --- a/internal/generate/clusterserviceversion/bases/markers.go +++ b/internal/generate/clusterserviceversion/bases/markers.go @@ -21,31 +21,32 @@ import ( "github.com/markbates/inflect" "github.com/operator-framework/api/pkg/operators/v1alpha1" + "github.com/operator-framework/operator-registry/pkg/registry" log "github.com/sirupsen/logrus" "k8s.io/apimachinery/pkg/runtime/schema" + "github.com/operator-framework/operator-sdk/internal/generate/clusterserviceversion/bases/definitions" "github.com/operator-framework/operator-sdk/internal/generate/olm-catalog/descriptor" ) -// updateDescriptionsForGVKs updates csv with API metadata found in apisDir -// filtered by gvks. -func updateDescriptionsForGVKs(csv *v1alpha1.ClusterServiceVersion, apisDir string, - gvks []schema.GroupVersionKind) error { - - gvkMap := make(map[schema.GroupVersionKind]v1alpha1.CRDDescription) - for _, desc := range csv.Spec.CustomResourceDefinitions.Owned { - group := desc.Name - if split := strings.Split(desc.Name, "."); len(split) > 1 { - group = strings.Join(split[1:], ".") +// updateDefinitions parses APIs in apisDir for code and markers that can build a crdDescription and +// updates existing crdDescriptions in csv. If no code/markers are found, the crdDescription is appended as-is. +func updateDefinitions(csv *v1alpha1.ClusterServiceVersion, apisDir string, gvks []schema.GroupVersionKind) error { + keys := make([]registry.DefinitionKey, len(gvks)) + for i, gvk := range gvks { + keys[i] = registry.DefinitionKey{ + Name: fmt.Sprintf("%s.%s", inflect.Pluralize(strings.ToLower(gvk.Kind)), gvk.Group), + Group: gvk.Group, + Version: gvk.Version, + Kind: gvk.Kind, } - // Parse CRD descriptors from source code comments and annotations. - gvk := schema.GroupVersionKind{ - Group: group, - Version: desc.Version, - Kind: desc.Kind, - } - gvkMap[gvk] = desc } + return definitions.ApplyDefinitionsForKeysGo(csv, apisDir, keys) +} + +// updateDescriptionsForGVKs updates csv with API metadata found in apisDir filtered by gvks. +func updateDescriptionsForGVKs(csv *v1alpha1.ClusterServiceVersion, apisDir string, + gvks []schema.GroupVersionKind) error { descriptions := []v1alpha1.CRDDescription{} for _, gvk := range gvks { @@ -60,15 +61,12 @@ func updateDescriptionsForGVKs(csv *v1alpha1.ClusterServiceVersion, apisDir stri // like we do for the above cases. return fmt.Errorf("failed to set CRD descriptors for %s: %v", gvk, err) } - // Keep the existing description and don't update on error - if desc, hasDesc := gvkMap[gvk]; hasDesc { - descriptions = append(descriptions, desc) - } - } else { - // Replace the existing description with the newly parsed one - newDescription.Name = inflect.Pluralize(strings.ToLower(gvk.Kind)) + "." + gvk.Group - descriptions = append(descriptions, newDescription) + continue } + + // Replace the existing description with the newly parsed one + newDescription.Name = inflect.Pluralize(strings.ToLower(gvk.Kind)) + "." + gvk.Group + descriptions = append(descriptions, newDescription) } csv.Spec.CustomResourceDefinitions.Owned = descriptions return nil diff --git a/internal/generate/crd/crd_test.go b/internal/generate/crd/crd_test.go index d20bafbf69..6b62fd06e1 100644 --- a/internal/generate/crd/crd_test.go +++ b/internal/generate/crd/crd_test.go @@ -31,15 +31,17 @@ import ( ) const ( - testGroup = "cache.example.com" - testVersion = "v1alpha1" - testKind = "Memcached" + testGroup = "cache" + testFullGroup = testGroup + ".example.com" + testVersion = "v1alpha1" + testKind = "Memcached" ) var ( testDataDir = filepath.Join("..", "testdata") testGoDataDir = filepath.Join(testDataDir, "go") - testAPIVersion = path.Join(testGroup, testVersion) + testGoAPIDir = filepath.Join(testGoDataDir, "pkg", "apis", testGroup, testVersion) + testAPIVersion = path.Join(testFullGroup, testVersion) ) func init() { @@ -77,7 +79,7 @@ func TestGenerate(t *testing.T) { description: "Generate Go CRD", generator: Generator{ IsOperatorGo: true, - ApisDir: filepath.Join(testGoDataDir, scaffold.ApisDir), + ApisDir: testGoAPIDir, OutputDir: filepath.Join(tmp, randomString()), CRDVersion: "v1beta1", }, @@ -87,7 +89,7 @@ func TestGenerate(t *testing.T) { description: "Generate non-Go CRD", generator: Generator{ IsOperatorGo: false, - ApisDir: filepath.Join(testGoDataDir, scaffold.ApisDir), + ApisDir: testGoAPIDir, OutputDir: filepath.Join(tmp, randomString()), CRDVersion: "v1beta1", Resource: *r, @@ -98,7 +100,7 @@ func TestGenerate(t *testing.T) { description: "invalid Go CRD version", generator: Generator{ IsOperatorGo: true, - ApisDir: filepath.Join(testGoDataDir, scaffold.ApisDir), + ApisDir: testGoAPIDir, OutputDir: filepath.Join(tmp, randomString()), CRDVersion: "invalid", }, @@ -108,7 +110,7 @@ func TestGenerate(t *testing.T) { description: "invalid non-Go CRD version", generator: Generator{ IsOperatorGo: false, - ApisDir: filepath.Join(testGoDataDir, scaffold.ApisDir), + ApisDir: testGoAPIDir, OutputDir: filepath.Join(tmp, randomString()), CRDVersion: "invalid", Resource: *r, @@ -133,7 +135,7 @@ func TestGenerate(t *testing.T) { func TestCRDGo(t *testing.T) { g := Generator{ IsOperatorGo: true, - ApisDir: filepath.Join(testGoDataDir, scaffold.ApisDir), + ApisDir: testGoAPIDir, } r, err := scaffold.NewResource(testAPIVersion, testKind) diff --git a/internal/generate/testdata/clusterserviceversions/bases/with-ui-metadata.clusterserviceversion.yaml b/internal/generate/testdata/clusterserviceversions/bases/with-ui-metadata.clusterserviceversion.yaml index 281662bafa..a8dcf6eff4 100644 --- a/internal/generate/testdata/clusterserviceversions/bases/with-ui-metadata.clusterserviceversion.yaml +++ b/internal/generate/testdata/clusterserviceversions/bases/with-ui-metadata.clusterserviceversion.yaml @@ -18,8 +18,6 @@ spec: - description: Size is the size of the memcached deployment displayName: Size path: size - x-descriptors: - - urn:alm:descriptor:com.tectonic.ui:podCount statusDescriptors: - description: Nodes are the names of the memcached pods displayName: Nodes diff --git a/internal/generate/testdata/clusterserviceversions/newlayout/manifests/memcached-operator.clusterserviceversion.yaml b/internal/generate/testdata/clusterserviceversions/newlayout/manifests/memcached-operator.clusterserviceversion.yaml index d8377e4fd3..6c5a0b6a97 100644 --- a/internal/generate/testdata/clusterserviceversions/newlayout/manifests/memcached-operator.clusterserviceversion.yaml +++ b/internal/generate/testdata/clusterserviceversions/newlayout/manifests/memcached-operator.clusterserviceversion.yaml @@ -30,8 +30,6 @@ spec: - description: Size is the size of the memcached deployment displayName: Size path: size - x-descriptors: - - urn:alm:descriptor:com.tectonic.ui:podCount statusDescriptors: - description: Nodes are the names of the memcached pods displayName: Nodes diff --git a/internal/generate/testdata/go/api/v1alpha2/doc.go b/internal/generate/testdata/go/api/v1alpha2/doc.go new file mode 100644 index 0000000000..09ac980c8c --- /dev/null +++ b/internal/generate/testdata/go/api/v1alpha2/doc.go @@ -0,0 +1,18 @@ +// Copyright 2020 The Operator-SDK Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Package v1alpha1 contains API Schema definitions for the cache v1alpha1 API group +// +k8s:deepcopy-gen=package,register +// +groupName=cache.example.com +package v1alpha2 diff --git a/internal/generate/testdata/go/api/v1alpha2/dummy_types.go b/internal/generate/testdata/go/api/v1alpha2/dummy_types.go new file mode 100644 index 0000000000..9f70963c53 --- /dev/null +++ b/internal/generate/testdata/go/api/v1alpha2/dummy_types.go @@ -0,0 +1,173 @@ +// Copyright 2020 The Operator-SDK Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package v1alpha2 + +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// +k8s:deepcopy-gen=false +// +k8s:openapi-gen=false +type NoKindSpec struct { + // Not included in anything, no kind type + // +operator-sdk:csv:customresourcedefinitions:type=spec + // +operator-sdk:csv:customresourcedefinitions:type=status + Size int32 `json:"size"` + // Not included in anything, no kind type + Boss Hog `json:"hog"` +} + +// +k8s:deepcopy-gen=false +// +k8s:openapi-gen=false +type NoKindStatus struct { + // Not included in anything, no kind type + // +operator-sdk:csv:customresourcedefinitions:type=spec + // +operator-sdk:csv:customresourcedefinitions:type=status + Nodes []string `json:"nodes"` +} + +// +k8s:deepcopy-gen=false +// +k8s:openapi-gen=false +type DummySpec struct { + // Should be in spec + // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="dummy-size",xDescriptors="urn:alm:descriptor:com.tectonic.ui:podCount" + Size int32 `json:"size"` + // Should be in spec, but should not have array index in path + // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Wheels",xDescriptors="urn:alm:descriptor:com.tectonic.ui:text" + Wheels []Wheel `json:"wheels"` +} + +// +k8s:deepcopy-gen=false +// +k8s:openapi-gen=false +type DummyStatus struct { + // Should be in status but not spec, since DummyStatus isn't in DummySpec + // +operator-sdk:csv:customresourcedefinitions:type=spec + // +operator-sdk:csv:customresourcedefinitions:type=status + Nodes []string `json:"nodes"` + // Not included in status but children should be + Boss Hog `json:"hog"` +} + +// +k8s:deepcopy-gen=false +// +k8s:openapi-gen=false +type Hog struct { + // Should be in status but not spec, since Hog isn't in DummySpec + // +operator-sdk:csv:customresourcedefinitions:type=spec + // +operator-sdk:csv:customresourcedefinitions:type=status,displayName="boss-hog-engine" + Engine Engine `json:"engine"` + // Not in spec or status, no boolean annotation + // +operator-sdk:csv:customresourcedefinitions:displayName="doesnt-matter" + Brand string `json:"brand"` + // Not in spec or status + Helmet string `json:"helmet"` + // Fields should be inlined + // +operator-sdk:csv:customresourcedefinitions:type=spec + // +operator-sdk:csv:customresourcedefinitions:type=status + Inlined InlinedComponent `json:",inline"` + // Fields should be inlined + InlinedComponent `json:",inline"` + // Should be ignored + // +operator-sdk:csv:customresourcedefinitions:type=spec + // +operator-sdk:csv:customresourcedefinitions:type=status + Ignored IgnoredComponent `json:"-"` + // Should be ignored, but exported children should not be + notExported `json:",inline"` +} + +type notExported struct { + // +operator-sdk:csv:customresourcedefinitions:type=spec + // +operator-sdk:csv:customresourcedefinitions:type=status + Public string `json:"foo"` + // +operator-sdk:csv:customresourcedefinitions:type=spec + // +operator-sdk:csv:customresourcedefinitions:type=status + private string `json:"-"` +} + +// +k8s:deepcopy-gen=false +// +k8s:openapi-gen=false +type Engine struct { + // Should not be included, no annotations. + Pistons []string `json:"pistons"` +} + +// +k8s:deepcopy-gen=false +// +k8s:openapi-gen=false +type Wheel struct { + // Type should be in spec with path equal to wheels[0].type + // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Wheel Type",xDescriptors="urn:alm:descriptor:com.tectonic.ui:arrayFieldGroup:wheels" ; "urn:alm:descriptor:com.tectonic.ui:text" + Type string `json:"type"` +} + +// +k8s:deepcopy-gen=false +// +k8s:openapi-gen=false +type InlinedComponent struct { + // +operator-sdk:csv:customresourcedefinitions:type=spec + // +operator-sdk:csv:customresourcedefinitions:type=status + SeatMaterial string `json:"seatMaterial"` +} + +// +k8s:deepcopy-gen=false +// +k8s:openapi-gen=false +type IgnoredComponent struct { + // +operator-sdk:csv:customresourcedefinitions:type=spec + // +operator-sdk:csv:customresourcedefinitions:type=status + TrunkSpace string `json:"trunkSpace"` +} + +// +k8s:deepcopy-gen=false +// +k8s:openapi-gen=false +type OtherDummyStatus struct { + // Should be in status but not spec, since this isn't a spec type + // +operator-sdk:csv:customresourcedefinitions:type=spec + // +operator-sdk:csv:customresourcedefinitions:type=status + Nothing string `json:"nothing"` +} + +// Dummy is the Schema for the dummy API +// +k8s:deepcopy-gen=false +// +k8s:openapi-gen=false +// +kubebuilder:subresource:status +// +kubebuilder:resource:path=dummys,scope=Namespaced +// +operator-sdk:csv:customresourcedefinitions:displayName="Dummy App" +// +operator-sdk:csv:customresourcedefinitions:resources={{Deployment,v1,"dummy-deployment"},{ReplicaSet,v1beta2,"dummy-replicaset"},{Pod,v1,"dummy-pod"}} +type Dummy struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` + + Spec DummySpec `json:"spec,omitempty"` + Status DummyStatus `json:"status,omitempty"` +} + +// OtherDummy is the Schema for the other dummy API +// +k8s:deepcopy-gen=false +// +k8s:openapi-gen=false +// +operator-sdk:csv:customresourcedefinitions:displayName="Other Dummy App" +// +operator-sdk:csv:customresourcedefinitions:resources={{Service,v1,"other-dummy-service"},{Pod,v1,"other-dummy-pod"}} +type OtherDummy struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` + + Spec Hog `json:"spec,omitempty"` + Status OtherDummyStatus `json:"status,omitempty"` +} + +// DummyList contains a list of Dummy +// +k8s:deepcopy-gen=false +// +k8s:openapi-gen=false +type DummyList struct { + metav1.TypeMeta `json:",inline"` + metav1.ListMeta `json:"metadata,omitempty"` + Items []Dummy `json:"items"` +} diff --git a/internal/generate/testdata/go/api/v1alpha2/memcached_types.go b/internal/generate/testdata/go/api/v1alpha2/memcached_types.go new file mode 100644 index 0000000000..28c8588f9c --- /dev/null +++ b/internal/generate/testdata/go/api/v1alpha2/memcached_types.go @@ -0,0 +1,57 @@ +// Copyright 2020 The Operator-SDK Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package v1alpha2 + +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// MemcachedSpec defines the desired state of Memcached +type MemcachedSpec struct { + // Size is the size of the memcached deployment + // +operator-sdk:csv:customresourcedefinitions:type=spec + Size int32 `json:"size"` +} + +// MemcachedStatus defines the observed state of Memcached +type MemcachedStatus struct { + // Nodes are the names of the memcached pods + // +operator-sdk:csv:customresourcedefinitions:type=status + Nodes []string `json:"nodes"` +} + +// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object + +// Memcached is the Schema for the memcacheds API +// +kubebuilder:subresource:status +// +kubebuilder:resource:path=memcacheds,scope=Namespaced +// +kubebuilder:storageversion +// +operator-sdk:csv:customresourcedefinitions:displayName="Memcached App" +type Memcached struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` + + Spec MemcachedSpec `json:"spec,omitempty"` + Status MemcachedStatus `json:"status,omitempty"` +} + +// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object + +// MemcachedList contains a list of Memcached +type MemcachedList struct { + metav1.TypeMeta `json:",inline"` + metav1.ListMeta `json:"metadata,omitempty"` + Items []Memcached `json:"items"` +} diff --git a/internal/generate/testdata/go/api/v1alpha2/memcachedrs_types.go b/internal/generate/testdata/go/api/v1alpha2/memcachedrs_types.go new file mode 100644 index 0000000000..196ff59c6b --- /dev/null +++ b/internal/generate/testdata/go/api/v1alpha2/memcachedrs_types.go @@ -0,0 +1,55 @@ +// Copyright 2020 The Operator-SDK Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package v1alpha2 + +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// MemcachedRSSpec defines the desired state of MemcachedRS +type MemcachedRSSpec struct { + // +operator-sdk:csv:customresourcedefinitions:type=spec + NumNodes int32 `json:"numNodes"` +} + +// MemcachedRSStatus defines the observed state of MemcachedRS +type MemcachedRSStatus struct { + // +operator-sdk:csv:customresourcedefinitions:type=status + NodeList []string `json:"nodeList"` +} + +// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object + +// MemcachedRS is the Schema for the memcachedrs API +// +kubebuilder:subresource:status +// +kubebuilder:resource:path=memcachedrs,scope=Namespaced +// +kubebuilder:storageversion +// +operator-sdk:csv:customresourcedefinitions:displayName="MemcachedRS App" +type MemcachedRS struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` + + Spec MemcachedRSSpec `json:"spec,omitempty"` + Status MemcachedRSStatus `json:"status,omitempty"` +} + +// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object + +// MemcachedRSList contains a list of MemcachedRS +type MemcachedRSList struct { + metav1.TypeMeta `json:",inline"` + metav1.ListMeta `json:"metadata,omitempty"` + Items []MemcachedRS `json:"items"` +} diff --git a/internal/generate/testdata/non-standard-layout/api/cache/v1alpha1/memcached_types.go b/internal/generate/testdata/non-standard-layout/api/cache/v1alpha1/memcached_types.go index 156d2bb86b..99b77521bf 100644 --- a/internal/generate/testdata/non-standard-layout/api/cache/v1alpha1/memcached_types.go +++ b/internal/generate/testdata/non-standard-layout/api/cache/v1alpha1/memcached_types.go @@ -21,14 +21,14 @@ import ( // MemcachedSpec defines the desired state of Memcached type MemcachedSpec struct { // Size is the size of the memcached deployment - // +operator-sdk:gen-csv:customresourcedefinitions.specDescriptors=true + // +operator-sdk:csv:customresourcedefinitions:type=spec Size int32 `json:"size"` } // MemcachedStatus defines the observed state of Memcached type MemcachedStatus struct { // Nodes are the names of the memcached pods - // +operator-sdk:gen-csv:customresourcedefinitions.statusDescriptors=true + // +operator-sdk:csv:customresourcedefinitions:type=status Nodes []string `json:"nodes"` } @@ -38,7 +38,7 @@ type MemcachedStatus struct { // +kubebuilder:subresource:status // +kubebuilder:resource:path=memcacheds,scope=Namespaced // +kubebuilder:storageversion -// +operator-sdk:gen-csv:customresourcedefinitions.displayName="Memcached App Display Name" +// +operator-sdk:csv:customresourcedefinitions:displayName="Memcached App Display Name" type Memcached struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` diff --git a/internal/generate/testdata/non-standard-layout/expected-catalog/olm-catalog/memcached-operator/0.0.1/memcached-operator.v0.0.1.clusterserviceversion.yaml b/internal/generate/testdata/non-standard-layout/expected-catalog/olm-catalog/memcached-operator/0.0.1/memcached-operator.v0.0.1.clusterserviceversion.yaml index d8377e4fd3..6c5a0b6a97 100644 --- a/internal/generate/testdata/non-standard-layout/expected-catalog/olm-catalog/memcached-operator/0.0.1/memcached-operator.v0.0.1.clusterserviceversion.yaml +++ b/internal/generate/testdata/non-standard-layout/expected-catalog/olm-catalog/memcached-operator/0.0.1/memcached-operator.v0.0.1.clusterserviceversion.yaml @@ -30,8 +30,6 @@ spec: - description: Size is the size of the memcached deployment displayName: Size path: size - x-descriptors: - - urn:alm:descriptor:com.tectonic.ui:podCount statusDescriptors: - description: Nodes are the names of the memcached pods displayName: Nodes diff --git a/internal/generate/testdata/non-standard-layout/expected-catalog/olm-catalog/memcached-operator/0.0.3/memcached-operator.v0.0.3.clusterserviceversion.yaml b/internal/generate/testdata/non-standard-layout/expected-catalog/olm-catalog/memcached-operator/0.0.3/memcached-operator.v0.0.3.clusterserviceversion.yaml index e5c1982b18..8d9dcde805 100644 --- a/internal/generate/testdata/non-standard-layout/expected-catalog/olm-catalog/memcached-operator/0.0.3/memcached-operator.v0.0.3.clusterserviceversion.yaml +++ b/internal/generate/testdata/non-standard-layout/expected-catalog/olm-catalog/memcached-operator/0.0.3/memcached-operator.v0.0.3.clusterserviceversion.yaml @@ -30,8 +30,6 @@ spec: - description: Size is the size of the memcached deployment displayName: Size path: size - x-descriptors: - - urn:alm:descriptor:com.tectonic.ui:podCount statusDescriptors: - description: Nodes are the names of the memcached pods displayName: Nodes diff --git a/internal/generate/testdata/non-standard-layout/expected-catalog/olm-catalog/memcached-operator/0.0.4/memcached-operator.v0.0.4.clusterserviceversion.yaml b/internal/generate/testdata/non-standard-layout/expected-catalog/olm-catalog/memcached-operator/0.0.4/memcached-operator.v0.0.4.clusterserviceversion.yaml index 6972ee00bb..63f288cdb1 100644 --- a/internal/generate/testdata/non-standard-layout/expected-catalog/olm-catalog/memcached-operator/0.0.4/memcached-operator.v0.0.4.clusterserviceversion.yaml +++ b/internal/generate/testdata/non-standard-layout/expected-catalog/olm-catalog/memcached-operator/0.0.4/memcached-operator.v0.0.4.clusterserviceversion.yaml @@ -30,8 +30,6 @@ spec: - description: Size is the size of the memcached deployment displayName: Size path: size - x-descriptors: - - urn:alm:descriptor:com.tectonic.ui:podCount statusDescriptors: - description: Nodes are the names of the memcached pods displayName: Nodes diff --git a/internal/markers/markers.go b/internal/markers/markers.go new file mode 100644 index 0000000000..e05f6707ab --- /dev/null +++ b/internal/markers/markers.go @@ -0,0 +1,19 @@ +// Copyright 2019 The Operator-SDK Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package markers + +const ( + Prefix = "operator-sdk" +) diff --git a/website/content/en/docs/building-operators/golang/references/markers.md b/website/content/en/docs/building-operators/golang/references/markers.md index d737965cb4..b3e4597862 100644 --- a/website/content/en/docs/building-operators/golang/references/markers.md +++ b/website/content/en/docs/building-operators/golang/references/markers.md @@ -8,11 +8,153 @@ This document describes [code markers][markers] supported by the SDK. ## ClusterServiceVersion markers -This section details ClusterServiceVersion (CSV) [code markers][code-markers-design] and lists available markers. +This section details ClusterServiceVersion (CSV) code markers and lists available markers. **Note:** CSV markers can only be used in Go Operator projects. Annotations for Ansible and Helm Operator projects will be added in the future. -## Usage +### Usage + +All CSV markers have the prefix `+operator-sdk:csv`. + +#### `+operator-sdk:csv:customresourcedefinitions` + +These markers populate [owned `customresourcedefinitions`][csv-crds] in your CSV. + +Possible type-level markers: +- `+operator-sdk:csv:customresourcedefinitions:displayName="some display name"` + - Configures the kind's display name. +- `+operator-sdk:csv:customresourcedefinitions:resources={{Kind1,v1alpha1,"dns-name-1"},{Kind2,v1,"dns-name-2"},...}` + - Configures the kind's resources. + +Possible field-level markers, all of which must contain the `type=[spec,status]` key-value pair: +- `+operator-sdk:csv:customresourcedefinitions:type=[spec,status],displayName="some field display name"` + - Configures the field's display name. +- `+operator-sdk:csv:customresourcedefinitions:type=[spec,status],xDescriptors="urn:alm:descriptor:com.tectonic.ui:podCount,urn:alm:descriptor:io.kubernetes:custom"` + - Configures the field's x-descriptors. + + +Top-level `kind`, `name`, and `version` fields are parsed from API code. +All `description` fields are parsed from type declaration and `struct` type field comments. +All `path` fields are parsed from a field's JSON tag and merged with parent +field path's in dot-hierarchy notation. + +##### x-descriptors + +Check out the [descriptor reference][csv-x-desc] for available `x-descriptors` paths. + +#### Examples + +These examples assume `Memcached`, `MemcachedSpec`, and `MemcachedStatus` are the example projects' kind, spec, and status. + +1. Set a `displayName` and `resources` for a `customresourcedefinitions` kind entry: + + ```go + // +operator-sdk:csv:customresourcedefinitions:displayName="Memcached App",resources={{Pod,v1,memcached-runner},{Deployment,v1,memcached-deployment}} + type Memcached struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` + + Spec MemcachedSpec `json:"spec,omitempty"` + Status MemcachedStatus `json:"status,omitempty"` + } + ``` + +2. Set `displayName`, `path`, `xDescriptors`, and `description` on a field for a `customresourcedefinitions.specDescriptors` entry: + + ```go + type MemcachedSpec struct { + // Size is the size of the memcached deployment. <-- This will become Size's specDescriptors.description. + // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Number of pods",xDescriptors="urn:alm:descriptor:com.tectonic.ui:podCount,urn:alm:descriptor:io.kubernetes:custom" + Size int32 `json:"size"` // <-- Size's specDescriptors.path is inferred from this JSON tag. + } + ``` + +3. Let the SDK infer all unmarked paths on a field for a `customresourcedefinitions.specDescriptors` entry: + + ```go + type MemcachedSpec struct { + // Size is the size of the memcached deployment. + // +operator-sdk:csv:customresourcedefinitions:type=spec + Size int32 `json:"size"` + } + ``` + + The SDK uses the `Size` fields' `json` tag name as `path`, `Size` as `displayName`, and field comments as `description`. + +4. A comprehensive example: + - Infer `path`, `description`, `displayName`, and `x-descriptors` for `specDescriptors` and `statusDescriptors` entries. + - Create three `resources` entries each with `kind`, `version`, and `name` values. + + ```go + // Represents a cluster of Memcached apps + // +operator-sdk:csv:customresourcedefinitions:displayName="Memcached App",resources={{Pod,v1,memcached-runner},{Deployment,v1,memcached-deployment}} + type Memcached struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` + + Spec MemcachedSpec `json:"spec,omitempty"` + Status MemcachedStatus `json:"status,omitempty"` + } + + type MemcachedSpec struct { + Pods MemcachedPods `json:"pods"` + } + + type MemcachedStatus struct { + Pods MemcachedPods `json:"podStatuses"` + // +operator-sdk:csv:customresourcedefinitions:type=status,displayName="Pod Count",xDescriptors="urn:alm:descriptor:com.tectonic.ui:podCount" + PodCount int `json:"podCount"` + } + + type MemcachedPods struct { + // Size is the size of the memcached deployment. + // +operator-sdk:csv:customresourcedefinitions:type=spec + // +operator-sdk:csv:customresourcedefinitions.type=status + Size int32 `json:"size"` + } + ``` + + The generated `customresourcedefinitions` will look like: + + ```yaml + customresourcedefinitions: + owned: + - description: Represents a cluster of Memcached apps + displayName: Memcached App + kind: Memcached + name: memcacheds.cache.example.com + version: v1alpha1 + resources: + - kind: Deployment + name: memcached-deployment + version: v1 + - kind: Pod + name: memcached-runner + version: v1 + specDescriptors: + - description: The desired number of member Pods for the deployment. + displayName: Size + path: pods.size + statusDescriptors: + - description: The desired number of member Pods for the deployment. + displayName: Size + path: podStatuses.size + - displayName: Size + path: podCount + x-descriptors: + - 'urn:alm:descriptor:com.tectonic.ui:podCount' + ``` + + +## Deprecated markers + +_These markers are deprecated. You can migrate to the [new marker system](#clusterserviceversion-markers) by running the following script:_ + +```console +$ curl -sSLo migrate-markers.sh https://raw.githubusercontent.com/operator-framework/operator-sdk/master/hack/generate/migrate-markers.sh +$ chmod +x ./migrate-markers.sh +$ ./migrate-markers.sh path/to/*_types.go +``` All markers have a `+operator-sdk:gen-csv` prefix, denoting that they're parsed while executing [`operator-sdk generate kustomize manifests`][cli-gen-kustomize-manifests]. @@ -38,172 +180,9 @@ Example: `+operator-sdk:gen-csv:customresourcedefinitions.specDescriptors.displa - All `description` fields are parsed from type declaration and `struct` type field comments. - `path` is parsed out of a field's JSON tag and merged with parent field path's in dot-hierarchy notation. -### Examples - -These examples assume `Memcached`, `MemcachedSpec`, and `MemcachedStatus` are the example projects' kind, spec, and status. - -1. Set a display name for a `customresourcedefinitions` kind entry: - -```go -// +operator-sdk:gen-csv:customresourcedefinitions.displayName="Memcached App" -type Memcached struct { - metav1.TypeMeta `json:",inline"` - metav1.ObjectMeta `json:"metadata,omitempty"` - - Spec MemcachedSpec `json:"spec,omitempty"` - Status MemcachedStatus `json:"status,omitempty"` -} -``` - -2. Set `displayName`, `path`, `x-descriptors`, and `description` on a field for a `customresourcedefinitions.specDescriptors` entry: - -```go -type MemcachedSpec struct { - // Size is the size of the memcached deployment. <-- This will become Size's specDescriptors.description. - // +operator-sdk:gen-csv:customresourcedefinitions.specDescriptors=true - // +operator-sdk:gen-csv:customresourcedefinitions.specDescriptors.displayName="Pod Count" - // +operator-sdk:gen-csv:customresourcedefinitions.specDescriptors.x-descriptors="urn:alm:descriptor:com.tectonic.ui:podCount,urn:alm:descriptor:io.kubernetes:custom" - Size int32 `json:"size"` // <-- Size's specDescriptors.path is inferred from this JSON tag. -} -``` - -3. Let the SDK infer all unmarked paths on a field for a `customresourcedefinitions.specDescriptors` entry: - -```go -type MemcachedSpec struct { - // Size is the size of the memcached deployment. - // +operator-sdk:gen-csv:customresourcedefinitions.specDescriptors=true - Size int32 `json:"size"` -} -``` - -The SDK uses the `Size` fields' `json` tag name as `path`, `Size` as `displayName`, and field comments as `description`. - -The SDK also checks `path` elements against a list of well-known path to x-descriptor string mappings and either uses a match as `x-descriptors`, or does not set `x-descriptors`. Supported mappings: - -#### Spec x-descriptors - -{{}} -| Path | x-descriptor | -|-----|-----| -| `size` | `urn:alm:descriptor:com.tectonic.ui:podCount` | -| `podCount` | `urn:alm:descriptor:com.tectonic.ui:podCount` | -| `endpoints` | `urn:alm:descriptor:com.tectonic.ui:endpointList` | -| `endpointList` | `urn:alm:descriptor:com.tectonic.ui:endpointList` | -| `label` | `urn:alm:descriptor:com.tectonic.ui:label` | -| `resources` | `urn:alm:descriptor:com.tectonic.ui:resourceRequirements` | -| `resourceRequirements` | `urn:alm:descriptor:com.tectonic.ui:resourceRequirements` | -| `selector` | `urn:alm:descriptor:com.tectonic.ui:selector:` | -| `namespaceSelector` | `urn:alm:descriptor:com.tectonic.ui:namespaceSelector` | -| none | `urn:alm:descriptor:io.kubernetes:` | -| `booleanSwitch` | `urn:alm:descriptor:com.tectonic.ui:booleanSwitch` | -| `password` | `urn:alm:descriptor:com.tectonic.ui:password` | -| `checkbox` | `urn:alm:descriptor:com.tectonic.ui:checkbox` | -| `imagePullPolicy` | `urn:alm:descriptor:com.tectonic.ui:imagePullPolicy` | -| `updateStrategy` | `urn:alm:descriptor:com.tectonic.ui:updateStrategy` | -| `text` | `urn:alm:descriptor:com.tectonic.ui:text` | -| `number` | `urn:alm:descriptor:com.tectonic.ui:number` | -| `nodeAffinity` | `urn:alm:descriptor:com.tectonic.ui:nodeAffinity` | -| `podAffinity` | `urn:alm:descriptor:com.tectonic.ui:podAffinity` | -| `podAntiAffinity` | `urn:alm:descriptor:com.tectonic.ui:podAntiAffinity` | -| none | `urn:alm:descriptor:com.tectonic.ui:fieldGroup:` | -| none | `urn:alm:descriptor:com.tectonic.ui:arrayFieldGroup:` | -| none | `urn:alm:descriptor:com.tectonic.ui:select:` | -| `advanced` | `urn:alm:descriptor:com.tectonic.ui:advanced` | -{{
}} - -#### Status x-descriptors - -{{}} -| Path | x-descriptor | -|-----|-----| -| `podStatuses` | `urn:alm:descriptor:com.tectonic.ui:podStatuses` | -| `size` | `urn:alm:descriptor:com.tectonic.ui:podCount` | -| `podCount` | `urn:alm:descriptor:com.tectonic.ui:podCount` | -| `link` | `urn:alm:descriptor:org.w3:link` | -| `w3link` | `urn:alm:descriptor:org.w3:link` | -| `conditions` | `urn:alm:descriptor:io.kubernetes.conditions` | -| `text` | `urn:alm:descriptor:text` | -| `prometheusEndpoint` | `urn:alm:descriptor:prometheusEndpoint` | -| `phase` | `urn:alm:descriptor:io.kubernetes.phase` | -| `k8sPhase` | `urn:alm:descriptor:io.kubernetes.phase` | -| `reason` | `urn:alm:descriptor:io.kubernetes.phase:reason` | -| `k8sReason` | `urn:alm:descriptor:io.kubernetes.phase:reason` | -| none | `urn:alm:descriptor:io.kubernetes:` | -{{
}} - -**NOTE:** any x-descriptor that ends in `:` will not be inferred by `path` element, ex. `urn:alm:descriptor:io.kubernetes:`. Use the `x-descriptors` marker if you want to enable one for your type. - -4. A comprehensive example: -- Infer `path`, `description`, `displayName`, and `x-descriptors` for `specDescriptors` and `statusDescriptors` entries. -- Create three `resources` entries each with `kind`, `version`, and `name` values. - -```go -// Represents a cluster of Memcached apps -// +operator-sdk:gen-csv:customresourcedefinitions.displayName="Memcached App" -// +operator-sdk:gen-csv:customresourcedefinitions.resources="Deployment,v1,\"memcached-operator\"" -// +operator-sdk:gen-csv:customresourcedefinitions.resources=`Service,v1,"memcached-operator"` -type Memcached struct { - metav1.TypeMeta `json:",inline"` - metav1.ObjectMeta `json:"metadata,omitempty"` - - Spec MemcachedSpec `json:"spec,omitempty"` - Status MemcachedStatus `json:"status,omitempty"` -} - -type MemcachedSpec struct { - Pods MemcachedPods `json:"pods"` -} - -type MemcachedStatus struct { - Pods MemcachedPods `json:"podStatuses"` -} - -type MemcachedPods struct { - // Size is the size of the memcached deployment. - // +operator-sdk:gen-csv:customresourcedefinitions.specDescriptors=true - // +operator-sdk:gen-csv:customresourcedefinitions.statusDescriptors=true - Size int32 `json:"size"` -} -``` - -The generated `customresourcedefinitions` will look like: - -```yaml -customresourcedefinitions: - owned: - - description: Represents a cluster of Memcached apps - displayName: Memcached App - kind: Memcached - name: memcacheds.cache.example.com - version: v1alpha1 - resources: - - kind: Deployment - name: A Kubernetes Deployment - version: v1 - - kind: ReplicaSet - name: A Kubernetes ReplicaSet - version: v1beta2 - - kind: Pod - name: A Kubernetes Pod - version: v1 - specDescriptors: - - description: The desired number of member Pods for the deployment. - displayName: Size - path: pods.size - x-descriptors: - - 'urn:alm:descriptor:com.tectonic.ui:podCount' - statusDescriptors: - - description: The desired number of member Pods for the deployment. - displayName: Size - path: podStatuses.size - x-descriptors: - - 'urn:alm:descriptor:com.tectonic.ui:podStatuses' - - 'urn:alm:descriptor:com.tectonic.ui:podCount' -``` [markers]:https://pkg.go.dev/sigs.k8s.io/controller-tools/pkg/markers -[code-markers-design]:https://github.com/operator-framework/operator-sdk/blob/master/proposals/sdk-code-annotations.md [cli-gen-kustomize-manifests]:/docs/cli/operator-sdk_generate_kustomize_manifests -[csv-x-desc]:https://github.com/openshift/console/blob/feabd61/frontend/packages/operator-lifecycle-manager/src/components/descriptors/types.ts#L3-L39 +[csv-x-desc]:https://github.com/openshift/console/blob/master/frontend/packages/operator-lifecycle-manager/src/components/descriptors/reference/reference.md [csv-spec]:https://github.com/operator-framework/operator-lifecycle-manager/blob/e0eea22/doc/design/building-your-csv.md +[csv-crds]:https://github.com/operator-framework/operator-lifecycle-manager/blob/master/doc/design/building-your-csv.md#your-custom-resource-definitions From a2a1993d2d0d6b9470fc6db95f7df90ad8fd695a Mon Sep 17 00:00:00 2001 From: Eric Stroczynski Date: Wed, 22 Jul 2020 11:21:19 -0700 Subject: [PATCH 2/4] remove legacy markers --- go.mod | 1 - .../bases/clusterserviceversion.go | 48 +-- .../bases/definitions/ast.go | 2 +- .../bases/definitions/crd.go | 16 +- .../bases/definitions/definitions.go | 4 +- .../bases/definitions/definitions_test.go | 12 +- .../definitions/{parse.go => markers.go} | 18 +- .../{parse_test.go => markers_test.go} | 2 + .../clusterserviceversion/bases/markers.go | 73 ---- .../clusterserviceversion/bases/metadata.go | 20 + .../clusterserviceversion.go | 44 --- .../clusterserviceversion_test.go | 50 --- .../olm-catalog/descriptor/descriptor.go | 261 ------------- .../olm-catalog/descriptor/descriptor_test.go | 298 --------------- .../generate/olm-catalog/descriptor/parse.go | 323 ---------------- .../olm-catalog/descriptor/parse_test.go | 348 ------------------ .../generate/olm-catalog/descriptor/search.go | 149 -------- .../golang/references/markers.md | 28 +- 18 files changed, 65 insertions(+), 1632 deletions(-) rename internal/generate/clusterserviceversion/bases/definitions/{parse.go => markers.go} (91%) rename internal/generate/clusterserviceversion/bases/definitions/{parse_test.go => markers_test.go} (98%) delete mode 100644 internal/generate/clusterserviceversion/bases/markers.go delete mode 100644 internal/generate/olm-catalog/descriptor/descriptor.go delete mode 100644 internal/generate/olm-catalog/descriptor/descriptor_test.go delete mode 100644 internal/generate/olm-catalog/descriptor/parse.go delete mode 100644 internal/generate/olm-catalog/descriptor/parse_test.go delete mode 100644 internal/generate/olm-catalog/descriptor/search.go diff --git a/go.mod b/go.mod index d2d2d78ab4..fa3c649322 100644 --- a/go.mod +++ b/go.mod @@ -37,7 +37,6 @@ require ( k8s.io/apimachinery v0.18.2 k8s.io/cli-runtime v0.18.2 k8s.io/client-go v0.18.2 - k8s.io/gengo v0.0.0-20200114144118-36b2048a9120 k8s.io/klog v1.0.0 k8s.io/kubectl v0.18.2 rsc.io/letsencrypt v0.0.3 // indirect diff --git a/internal/generate/clusterserviceversion/bases/clusterserviceversion.go b/internal/generate/clusterserviceversion/bases/clusterserviceversion.go index d9a8415513..208726de10 100644 --- a/internal/generate/clusterserviceversion/bases/clusterserviceversion.go +++ b/internal/generate/clusterserviceversion/bases/clusterserviceversion.go @@ -15,13 +15,9 @@ package bases import ( - "bufio" "bytes" "fmt" "io/ioutil" - "os" - "path/filepath" - "strings" "github.com/operator-framework/api/pkg/operators/v1alpha1" log "github.com/sirupsen/logrus" @@ -29,9 +25,7 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "sigs.k8s.io/yaml" - "github.com/operator-framework/operator-sdk/internal/markers" "github.com/operator-framework/operator-sdk/internal/util/k8sutil" - kbutil "github.com/operator-framework/operator-sdk/internal/util/kubebuilder" "github.com/operator-framework/operator-sdk/internal/util/projutil" ) @@ -87,15 +81,10 @@ func (b ClusterServiceVersion) GetBase() (base *v1alpha1.ClusterServiceVersion, switch b.OperatorType { case projutil.OperatorTypeGo: // Update descriptions from the APIs dir. - // TODO(estroz): remove this condition once old annotations are deprecated. - if kbutil.HasProjectFile() || areAnnotationsMigrated(b.APIsDir) { - err = updateDefinitions(base, b.APIsDir, b.GVKs) - } else { - err = updateDescriptionsForGVKs(base, b.APIsDir, b.GVKs) - } - if err != nil { - return nil, fmt.Errorf("error generating ClusterServiceVersion base metadata: %w", err) - } + err = updateDefinitions(base, b.APIsDir, b.GVKs) + } + if err != nil { + return nil, fmt.Errorf("error generating ClusterServiceVersion definitions metadata: %w", err) } } @@ -210,32 +199,3 @@ func readClusterServiceVersionBase(path string) (*v1alpha1.ClusterServiceVersion return nil, fmt.Errorf("no ClusterServiceVersion manifest in %s", path) } - -// areAnnotationsMigrated returns true if annotations have been migrated to the new markers format. -// Errors are ignored so the caller can default to legacy behavior. -func areAnnotationsMigrated(apisRoot string) bool { - if apisRoot == "" { - return false - } - hasMarkers := false - _ = filepath.Walk(apisRoot, func(path string, info os.FileInfo, err error) error { - if err != nil || info.IsDir() || filepath.Ext(path) != ".go" { - return err - } - f, err := os.Open(path) - if err != nil { - return err - } - scanner := bufio.NewScanner(f) - for scanner.Scan() { - // The new prefix "operator-sdk:csv:" differs from the old one, "operator-sdk:gen-csv:". - if strings.Contains(scanner.Text(), markers.Prefix+":csv:") { - hasMarkers = true - break - } - } - _ = f.Close() - return scanner.Err() - }) - return hasMarkers -} diff --git a/internal/generate/clusterserviceversion/bases/definitions/ast.go b/internal/generate/clusterserviceversion/bases/definitions/ast.go index cd4418b975..6ecbb08cf1 100644 --- a/internal/generate/clusterserviceversion/bases/definitions/ast.go +++ b/internal/generate/clusterserviceversion/bases/definitions/ast.go @@ -23,7 +23,7 @@ import ( "sigs.k8s.io/controller-tools/pkg/markers" ) -// getMarkedChildrenOfField collects all marked fields from type delcarations starting at root in depth-first order. +// getMarkedChildrenOfField collects all marked fields from type declarations starting at root in depth-first order. func (g generator) getMarkedChildrenOfField(root markers.FieldInfo) (map[string][]*fieldInfo, error) { // ast.Inspect will not traverse into fields, so iteratively collect them and to check for markers. nextFields := []*fieldInfo{{FieldInfo: root}} diff --git a/internal/generate/clusterserviceversion/bases/definitions/crd.go b/internal/generate/clusterserviceversion/bases/definitions/crd.go index 67bd8ae798..3933bcd516 100644 --- a/internal/generate/clusterserviceversion/bases/definitions/crd.go +++ b/internal/generate/clusterserviceversion/bases/definitions/crd.go @@ -21,9 +21,11 @@ import ( "strings" "github.com/fatih/structtag" + "github.com/markbates/inflect" "github.com/operator-framework/api/pkg/operators/v1alpha1" "github.com/operator-framework/operator-registry/pkg/registry" "k8s.io/apimachinery/pkg/version" + crdmarkers "sigs.k8s.io/controller-tools/pkg/crd/markers" "sigs.k8s.io/controller-tools/pkg/markers" "github.com/operator-framework/operator-sdk/internal/util/k8sutil" @@ -71,8 +73,7 @@ func getHalfBySep(s, sep string, half uint) string { // buildCRDDescriptionFromType builds a crdDescription for the Go API defined // by key from markers and type information in g.types. -func (g generator) buildCRDDescriptionFromType(key registry.DefinitionKey, - kindType *markers.TypeInfo) (v1alpha1.CRDDescription, error) { +func (g generator) buildCRDDescriptionFromType(key registry.DefinitionKey, kindType *markers.TypeInfo) (v1alpha1.CRDDescription, error) { // Initialize the description. description := MakeCRDDescriptionForKey(key) @@ -81,7 +82,8 @@ func (g generator) buildCRDDescriptionFromType(key registry.DefinitionKey, // Parse resources and displayName from the kind type's markers. for _, markers := range kindType.Markers { for _, marker := range markers { - if d, isDescription := marker.(Description); isDescription { + switch d := marker.(type) { + case Description: if d.DisplayName != "" { description.DisplayName = d.DisplayName } @@ -92,9 +94,17 @@ func (g generator) buildCRDDescriptionFromType(key registry.DefinitionKey, } description.Resources = append(description.Resources, refs...) } + case crdmarkers.Resource: + if d.Path != "" { + description.Name = fmt.Sprintf("%s.%s", d.Path, key.Group) + } } } } + // The default, if the resource marker's path value is not set, is to use a pluralized form of lowercase kind. + if description.Name == "" { + description.Name = fmt.Sprintf("%s.%s", inflect.Pluralize(strings.ToLower(key.Kind)), key.Group) + } sortDescription(description.Resources) // Find spec and status in the kind type. diff --git a/internal/generate/clusterserviceversion/bases/definitions/definitions.go b/internal/generate/clusterserviceversion/bases/definitions/definitions.go index 0366556689..ff48f1153c 100644 --- a/internal/generate/clusterserviceversion/bases/definitions/definitions.go +++ b/internal/generate/clusterserviceversion/bases/definitions/definitions.go @@ -16,7 +16,6 @@ package definitions import ( "errors" - "fmt" "os" "path/filepath" @@ -66,11 +65,10 @@ func ApplyDefinitionsForKeysGo(csv *v1alpha1.ClusterServiceVersion, apisRootDir // Create definitions for kind types found under the collected roots. definitionsByKey := make(map[registry.DefinitionKey]*descriptionValues) - fmt.Println("building types for:", keys) for _, key := range keys { kindType, hasKind := g.types[key.Kind] if !hasKind { - log.Debugf("Skipping CSV annotation parsing for API %s: type %s not found", key, key.Kind) + log.Warnf("Skipping CSV annotation parsing for API %s: type %s not found", key, key.Kind) continue } crd, err := g.buildCRDDescriptionFromType(key, kindType) diff --git a/internal/generate/clusterserviceversion/bases/definitions/definitions_test.go b/internal/generate/clusterserviceversion/bases/definitions/definitions_test.go index af3dd2a933..7064a928ea 100644 --- a/internal/generate/clusterserviceversion/bases/definitions/definitions_test.go +++ b/internal/generate/clusterserviceversion/bases/definitions/definitions_test.go @@ -24,6 +24,8 @@ import ( "github.com/stretchr/testify/assert" ) +// TODO(estroz): migrate to ginkgo/gomega + var ( testDataDir = filepath.Join("..", "..", "..", "testdata", "go") ) @@ -57,12 +59,12 @@ func TestApplyDefinitionsForKeysGo(t *testing.T) { apisDir: "api", csv: &v1alpha1.ClusterServiceVersion{}, keys: []registry.DefinitionKey{ - {Name: "dummies.cache.example.com", Group: "cache.example.com", Version: "v1alpha2", Kind: "Dummy"}, + {Name: "dummys.cache.example.com", Group: "cache.example.com", Version: "v1alpha2", Kind: "Dummy"}, }, expectedCRDs: v1alpha1.CustomResourceDefinitions{ Owned: []v1alpha1.CRDDescription{ { - Name: "dummies.cache.example.com", + Name: "dummys.cache.example.com", Kind: "Dummy", Version: "v1alpha2", DisplayName: "Dummy App", @@ -140,7 +142,7 @@ func TestApplyDefinitionsForKeysGo(t *testing.T) { CustomResourceDefinitions: v1alpha1.CustomResourceDefinitions{ Owned: []v1alpha1.CRDDescription{ { - Name: "dummies.cache.example.com", Version: "v1alpha2", Kind: "Dummy", + Name: "dummys.cache.example.com", Version: "v1alpha2", Kind: "Dummy", DisplayName: "Dummy App", Description: "Dummy is the Schema for the other dummy API", Resources: []v1alpha1.APIResourceReference{ @@ -160,12 +162,12 @@ func TestApplyDefinitionsForKeysGo(t *testing.T) { }, }, keys: []registry.DefinitionKey{ - {Name: "dummies.cache.example.com", Group: "cache.example.com", Version: "v1alpha2", Kind: "Dummy"}, + {Name: "dummys.cache.example.com", Group: "cache.example.com", Version: "v1alpha2", Kind: "Dummy"}, }, expectedCRDs: v1alpha1.CustomResourceDefinitions{ Owned: []v1alpha1.CRDDescription{ { - Name: "dummies.cache.example.com", Version: "v1alpha2", Kind: "Dummy", + Name: "dummys.cache.example.com", Version: "v1alpha2", Kind: "Dummy", DisplayName: "Dummy App", Description: "Dummy is the Schema for the other dummy API", Resources: []v1alpha1.APIResourceReference{ diff --git a/internal/generate/clusterserviceversion/bases/definitions/parse.go b/internal/generate/clusterserviceversion/bases/definitions/markers.go similarity index 91% rename from internal/generate/clusterserviceversion/bases/definitions/parse.go rename to internal/generate/clusterserviceversion/bases/definitions/markers.go index 9239203144..2aee5b5f57 100644 --- a/internal/generate/clusterserviceversion/bases/definitions/parse.go +++ b/internal/generate/clusterserviceversion/bases/definitions/markers.go @@ -22,6 +22,7 @@ import ( "github.com/fatih/structtag" "github.com/operator-framework/api/pkg/operators/v1alpha1" + crdmarkers "sigs.k8s.io/controller-tools/pkg/crd/markers" "sigs.k8s.io/controller-tools/pkg/markers" sdkmarkers "github.com/operator-framework/operator-sdk/internal/markers" @@ -41,6 +42,9 @@ var typeDefinition = markers.Must(markers.MakeDefinition(crdMarkerName, markers. // +operator-sdk:csv:customresourcedefinitions:type=,displayName="name",xDescriptors="ui:elements:foo:bar" var fieldDefinition = markers.Must(markers.MakeDefinition(crdMarkerName, markers.DescribesField, Descriptor{})) +// See https://github.com/kubernetes-sigs/controller-tools/blob/92e95c1/pkg/crd/markers/crd.go#L40 +var crdResourceDefinition = markers.Must(markers.MakeDefinition("kubebuilder:resource", markers.DescribesType, crdmarkers.Resource{})) + // registerMarkers adds type and field marker definitions to a registry. func registerMarkers(into *markers.Registry) error { if err := into.Register(typeDefinition); err != nil { @@ -51,6 +55,12 @@ func registerMarkers(into *markers.Registry) error { return fmt.Errorf("error registering field definition: %v", err) } into.AddHelp(fieldDefinition, Descriptor{}.Help()) + + // External definitions. + if err := into.Register(crdResourceDefinition); err != nil { + return fmt.Errorf("error registering CRD resource definition: %v", err) + } + into.AddHelp(crdResourceDefinition, crdmarkers.Resource{}.Help()) return nil } @@ -200,7 +210,7 @@ func getPathSegmentForField(finfo markers.FieldInfo) (string, error) { return inlinedTag, nil } // Unexported fields should be ignored in downstream processing. - if isNotExported(finfo.Name) { + if !isExported(finfo.Name) { return ignoredTag, nil } tags, err := structtag.Parse(string(finfo.Tag)) @@ -226,9 +236,9 @@ func getPathSegmentForField(finfo markers.FieldInfo) (string, error) { return finfo.Name, nil } -// isNotExported returns true if name is not an exported struct field name. -func isNotExported(name string) bool { - return len(name) == 0 || unicode.IsLower(rune(name[0])) +// isExported returns true if name is an exported struct field name. +func isExported(name string) bool { + return len(name) != 0 && !unicode.IsLower(rune(name[0])) } func contains(options []string, key string) bool { diff --git a/internal/generate/clusterserviceversion/bases/definitions/parse_test.go b/internal/generate/clusterserviceversion/bases/definitions/markers_test.go similarity index 98% rename from internal/generate/clusterserviceversion/bases/definitions/parse_test.go rename to internal/generate/clusterserviceversion/bases/definitions/markers_test.go index 08a334aa86..b636d5881b 100644 --- a/internal/generate/clusterserviceversion/bases/definitions/parse_test.go +++ b/internal/generate/clusterserviceversion/bases/definitions/markers_test.go @@ -21,6 +21,8 @@ import ( "github.com/operator-framework/api/pkg/operators/v1alpha1" ) +// TODO(estroz): migrate to ginkgo/gomega + func TestParseResource(t *testing.T) { cases := []struct { description string diff --git a/internal/generate/clusterserviceversion/bases/markers.go b/internal/generate/clusterserviceversion/bases/markers.go deleted file mode 100644 index 9565525124..0000000000 --- a/internal/generate/clusterserviceversion/bases/markers.go +++ /dev/null @@ -1,73 +0,0 @@ -// Copyright 2020 The Operator-SDK Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package bases - -import ( - "errors" - "fmt" - "strings" - - "github.com/markbates/inflect" - "github.com/operator-framework/api/pkg/operators/v1alpha1" - "github.com/operator-framework/operator-registry/pkg/registry" - log "github.com/sirupsen/logrus" - "k8s.io/apimachinery/pkg/runtime/schema" - - "github.com/operator-framework/operator-sdk/internal/generate/clusterserviceversion/bases/definitions" - "github.com/operator-framework/operator-sdk/internal/generate/olm-catalog/descriptor" -) - -// updateDefinitions parses APIs in apisDir for code and markers that can build a crdDescription and -// updates existing crdDescriptions in csv. If no code/markers are found, the crdDescription is appended as-is. -func updateDefinitions(csv *v1alpha1.ClusterServiceVersion, apisDir string, gvks []schema.GroupVersionKind) error { - keys := make([]registry.DefinitionKey, len(gvks)) - for i, gvk := range gvks { - keys[i] = registry.DefinitionKey{ - Name: fmt.Sprintf("%s.%s", inflect.Pluralize(strings.ToLower(gvk.Kind)), gvk.Group), - Group: gvk.Group, - Version: gvk.Version, - Kind: gvk.Kind, - } - } - return definitions.ApplyDefinitionsForKeysGo(csv, apisDir, keys) -} - -// updateDescriptionsForGVKs updates csv with API metadata found in apisDir filtered by gvks. -func updateDescriptionsForGVKs(csv *v1alpha1.ClusterServiceVersion, apisDir string, - gvks []schema.GroupVersionKind) error { - - descriptions := []v1alpha1.CRDDescription{} - for _, gvk := range gvks { - newDescription, err := descriptor.GetCRDDescriptionForGVK(apisDir, gvk) - if err != nil { - if errors.Is(err, descriptor.ErrAPIDirNotExist) { - log.Debugf("Directory for API %s does not exist. Skipping CSV annotation parsing for API.", gvk) - } else if errors.Is(err, descriptor.ErrAPITypeNotFound) { - log.Debugf("No kind type found for API %s. Skipping CSV annotation parsing for API.", gvk) - } else { - // TODO: Should we ignore all CSV annotation parsing errors and simply log the error - // like we do for the above cases. - return fmt.Errorf("failed to set CRD descriptors for %s: %v", gvk, err) - } - continue - } - - // Replace the existing description with the newly parsed one - newDescription.Name = inflect.Pluralize(strings.ToLower(gvk.Kind)) + "." + gvk.Group - descriptions = append(descriptions, newDescription) - } - csv.Spec.CustomResourceDefinitions.Owned = descriptions - return nil -} diff --git a/internal/generate/clusterserviceversion/bases/metadata.go b/internal/generate/clusterserviceversion/bases/metadata.go index 57917c3ee0..1015153550 100644 --- a/internal/generate/clusterserviceversion/bases/metadata.go +++ b/internal/generate/clusterserviceversion/bases/metadata.go @@ -15,10 +15,15 @@ package bases import ( + "fmt" "strings" + "github.com/markbates/inflect" "github.com/operator-framework/api/pkg/operators/v1alpha1" + "github.com/operator-framework/operator-registry/pkg/registry" + "k8s.io/apimachinery/pkg/runtime/schema" + "github.com/operator-framework/operator-sdk/internal/generate/clusterserviceversion/bases/definitions" "github.com/operator-framework/operator-sdk/internal/util/projutil" ) @@ -88,3 +93,18 @@ func (s uiMetadata) apply(csv *v1alpha1.ClusterServiceVersion) { csv.Spec.Provider = provider } } + +// updateDefinitions parses APIs in apisDir for code and markers that can build a crdDescription and +// updates existing crdDescriptions in csv. If no code/markers are found, the crdDescription is appended as-is. +func updateDefinitions(csv *v1alpha1.ClusterServiceVersion, apisDir string, gvks []schema.GroupVersionKind) error { + keys := make([]registry.DefinitionKey, len(gvks)) + for i, gvk := range gvks { + keys[i] = registry.DefinitionKey{ + Name: fmt.Sprintf("%s.%s", inflect.Pluralize(strings.ToLower(gvk.Kind)), gvk.Group), + Group: gvk.Group, + Version: gvk.Version, + Kind: gvk.Kind, + } + } + return definitions.ApplyDefinitionsForKeysGo(csv, apisDir, keys) +} diff --git a/internal/generate/clusterserviceversion/clusterserviceversion.go b/internal/generate/clusterserviceversion/clusterserviceversion.go index 5fb619ca6b..505d9d46ab 100644 --- a/internal/generate/clusterserviceversion/clusterserviceversion.go +++ b/internal/generate/clusterserviceversion/clusterserviceversion.go @@ -185,50 +185,6 @@ func (g Generator) setSDKAnnotations(csv *v1alpha1.ClusterServiceVersion) { csv.SetAnnotations(annotations) } -// LegacyOption is a function that modifies a Generator for legacy project layouts. -type LegacyOption Option - -// WithBundleBase sets a Generator's base CSV to a legacy-style bundle base. -func WithBundleBase(inputDir, apisDir string, ilvl projutil.InteractiveLevel) LegacyOption { - return func(g *Generator) error { - g.getBase = g.makeBundleBaseGetterLegacy(inputDir, apisDir, ilvl) - return nil - } -} - -// WithPackageBase sets a Generator's base CSV to a legacy-style package base. -func WithPackageBase(inputDir, apisDir string, ilvl projutil.InteractiveLevel) LegacyOption { - return func(g *Generator) error { - g.getBase = g.makePackageBaseGetterLegacy(inputDir, apisDir, ilvl) - return nil - } -} - -// GenerateLegacy configures the generator with opts then runs it. Used for -// generating files for legacy project layouts. -func (g *Generator) GenerateLegacy(opts ...LegacyOption) (err error) { - for _, opt := range opts { - if err = opt(g); err != nil { - return err - } - } - - if g.getWriter == nil { - return noGetWriterError - } - - csv, err := g.generate() - if err != nil { - return err - } - - w, err := g.getWriter() - if err != nil { - return err - } - return genutil.WriteObject(w, csv) -} - // generate runs a configured Generator. func (g *Generator) generate() (*operatorsv1alpha1.ClusterServiceVersion, error) { if g.getBase == nil { diff --git a/internal/generate/clusterserviceversion/clusterserviceversion_test.go b/internal/generate/clusterserviceversion/clusterserviceversion_test.go index 507580f177..491fb91a8b 100644 --- a/internal/generate/clusterserviceversion/clusterserviceversion_test.go +++ b/internal/generate/clusterserviceversion/clusterserviceversion_test.go @@ -198,39 +198,6 @@ var _ = Describe("Generating a ClusterServiceVersion", func() { Expect(outputFile).To(BeAnExistingFile()) Expect(readFileHelper(outputFile)).To(MatchYAML(newCSVStr)) }) - - It("should write a ClusterServiceVersion manifest to a legacy bundle file", func() { - g = Generator{ - OperatorName: operatorName, - OperatorType: operatorType, - Version: version, - Collector: col, - } - opts := []LegacyOption{ - WithBundleBase(csvBasesDir, goAPIsDir, projutil.InteractiveHardOff), - LegacyOption(WithBundleWriter(tmp)), - } - Expect(g.GenerateLegacy(opts...)).ToNot(HaveOccurred()) - outputFile := filepath.Join(tmp, bundle.ManifestsDir, makeCSVFileName(operatorName)) - Expect(outputFile).To(BeAnExistingFile()) - Expect(readFileHelper(outputFile)).To(MatchYAML(newCSVStr)) - }) - It("should write a ClusterServiceVersion manifest as a legacy package file", func() { - g = Generator{ - OperatorName: operatorName, - OperatorType: operatorType, - Version: version, - Collector: col, - } - opts := []LegacyOption{ - WithPackageBase(csvBasesDir, goAPIsDir, projutil.InteractiveHardOff), - LegacyOption(WithPackageWriter(tmp)), - } - Expect(g.GenerateLegacy(opts...)).Should(Succeed()) - outputFile := filepath.Join(tmp, g.Version, makeCSVFileName(operatorName)) - Expect(outputFile).To(BeAnExistingFile()) - Expect(string(readFileHelper(outputFile))).To(MatchYAML(newCSVStr)) - }) }) Context("with incorrect Options", func() { @@ -260,23 +227,6 @@ var _ = Describe("Generating a ClusterServiceVersion", func() { } Expect(g.Generate(cfg, opts...)).To(MatchError(noGetBaseError)) }) - - It("should return an error without any LegacyOptions", func() { - opts := []LegacyOption{} - Expect(g.GenerateLegacy(opts...)).To(MatchError(noGetWriterError)) - }) - It("should return an error without a getWriter (legacy)", func() { - opts := []LegacyOption{ - WithBundleBase(csvBasesDir, goAPIsDir, projutil.InteractiveHardOff), - } - Expect(g.GenerateLegacy(opts...)).To(MatchError(noGetWriterError)) - }) - It("should return an error without a getBase (legacy)", func() { - opts := []LegacyOption{ - LegacyOption(WithWriter(&bytes.Buffer{})), - } - Expect(g.GenerateLegacy(opts...)).To(MatchError(noGetBaseError)) - }) }) Context("to create a new base ClusterServiceVersion", func() { diff --git a/internal/generate/olm-catalog/descriptor/descriptor.go b/internal/generate/olm-catalog/descriptor/descriptor.go deleted file mode 100644 index 4177274349..0000000000 --- a/internal/generate/olm-catalog/descriptor/descriptor.go +++ /dev/null @@ -1,261 +0,0 @@ -// Copyright 2019 The Operator-SDK Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package descriptor - -import ( - "errors" - "fmt" - "os" - "path/filepath" - "strings" - - olmapiv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1" - log "github.com/sirupsen/logrus" - "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/gengo/parser" - "k8s.io/gengo/types" -) - -var ( - // ErrAPIDirNotExist is returned if an API directory does not exist. - ErrAPIDirNotExist = errors.New("directory for API does not exist") - // ErrAPITypeNotFound is returned if no type with a name matching the kind - // of an API is found. - ErrAPITypeNotFound = errors.New("kind type for API not found") -) - -// GetCRDDescriptionForGVK parses type and struct field declaration comments on -// API types to populate a csv's spec.customresourcedefinitions.owned fields -// for a given API identified by Group, Version, and Kind in apisDir. -// TODO(estroz): support ActionDescriptors parsing/setting. -func GetCRDDescriptionForGVK(apisDir string, gvk schema.GroupVersionKind) (olmapiv1alpha1.CRDDescription, error) { - crdDesc := olmapiv1alpha1.CRDDescription{ - Version: gvk.Version, - Kind: gvk.Kind, - } - group := gvk.Group - if strings.Contains(group, ".") { - group = strings.Split(group, ".")[0] - } - - // Check if apisDir exists - exists, err := isDirExist(apisDir) - if err != nil { - return olmapiv1alpha1.CRDDescription{}, err - } - if !exists { - log.Debugf("Could not find API types directory: %s", apisDir) - return olmapiv1alpha1.CRDDescription{}, ErrAPIDirNotExist - } - - // Check if the kind pkg is at the expected layout - // multi-group layout: // - // single-group layout: / - expectedPkgPath, err := getExpectedPkgLayout(apisDir, group, gvk.Version) - if err != nil { - return olmapiv1alpha1.CRDDescription{}, err - } - - // Get pkg types for the given GVK - var pkgTypes []*types.Type - if expectedPkgPath != "" { - // Look for the pkg types at the expected single or multi group import path - universe, err := getPkgsFromDirRecursive(expectedPkgPath) - if err != nil { - return olmapiv1alpha1.CRDDescription{}, err - } - pkgTypes, err = getTypesForPkgPath(expectedPkgPath, universe) - if err != nil { - return olmapiv1alpha1.CRDDescription{}, err - } - } else { - // Unknown apis directory layout: /.../ - // Look in recursively for expected pkg name - - // TODO: gengo.parse.AddDirRecursive() will (sometimes?) fail if the - // root apisDir has no .go files. - // Workaround for this is to have a doc.go file in the package. - // Move away from using gengo in the future if possible. - universe, err := getPkgsFromDirRecursive(apisDir) - if err != nil { - return olmapiv1alpha1.CRDDescription{}, err - } - pkgTypes, err = getTypesForPkgName(gvk.Version, universe) - if err != nil { - return olmapiv1alpha1.CRDDescription{}, err - } - } - - kindType := findKindType(gvk.Kind, pkgTypes) - if kindType == nil { - return olmapiv1alpha1.CRDDescription{}, ErrAPITypeNotFound - } - comments := append(kindType.SecondClosestCommentLines, kindType.CommentLines...) - kindDescriptors, err := parseCSVGenAnnotations(comments) - if err != nil { - return olmapiv1alpha1.CRDDescription{}, fmt.Errorf("error parsing CSV type %s annotations: %v", - kindType.Name.Name, err) - } - if description := parseDescription(comments); description != "" { - crdDesc.Description = description - } - if kindDescriptors.displayName != "" { - crdDesc.DisplayName = kindDescriptors.displayName - } - if len(kindDescriptors.resources) != 0 { - crdDesc.Resources = sortResources(kindDescriptors.resources) - } - for _, member := range kindType.Members { - path, err := getPathFromMember(member) - if err != nil { - return olmapiv1alpha1.CRDDescription{}, fmt.Errorf("error parsing %s type member %s JSON tags: %v", - gvk.Kind, member.Name, err) - } - if path != typeSpec && path != typeStatus { - continue - } - tree, err := newTypeTreeFromRoot(member.Type) - if err != nil { - return olmapiv1alpha1.CRDDescription{}, fmt.Errorf("error creating type tree for member type %s: %v", - member.Type.Name, err) - } - descriptors, err := tree.getDescriptorsFor(path) - if err != nil { - return olmapiv1alpha1.CRDDescription{}, err - } - if path == typeSpec { - for _, d := range sortDescriptors(descriptors) { - crdDesc.SpecDescriptors = append(crdDesc.SpecDescriptors, d.SpecDescriptor) - } - } else { - for _, d := range sortDescriptors(descriptors) { - crdDesc.StatusDescriptors = append(crdDesc.StatusDescriptors, olmapiv1alpha1.StatusDescriptor{ - Description: d.Description, - DisplayName: d.DisplayName, - Path: d.Path, - XDescriptors: d.XDescriptors, - }) - } - } - } - return crdDesc, nil -} - -func isDirExist(path string) (bool, error) { - fileInfo, err := os.Stat(path) - if err != nil { - if os.IsNotExist(err) { - return false, nil - } - return false, err - } - return fileInfo.IsDir(), nil -} - -// getExpectedPkgLayout checks the directory layout in apisDir -// for single and multi group layouts and returns the expected pkg path -// for the group and version. -// Returns empty string if neither single or multi group layout is detected -// multi group path: // -// single group path: / -func getExpectedPkgLayout(apisDir, group, version string) (expectedPkgPath string, err error) { - groupVersionDir := filepath.Join(apisDir, group, version) - if isMultiGroupLayout, err := isDirExist(groupVersionDir); isMultiGroupLayout { - if err != nil { - return "", err - } - return groupVersionDir, nil - } - versionDir := filepath.Join(apisDir, version) - if isSingleGroupLayout, err := isDirExist(versionDir); isSingleGroupLayout { - if err != nil { - return "", err - } - return versionDir, nil - } - // Neither multi nor single group layout - return "", nil -} - -// getPkgsFromDirRecursive gets all Go types from dir and recursively its sub directories. -// dir must be the project relative path to the pkg directory -func getPkgsFromDirRecursive(dir string) (types.Universe, error) { - if _, err := os.Stat(dir); err != nil { - return nil, err - } - p := parser.New() - // Gengo's AddDirRecursive fails to load subdir pkgs if the root dir - // isn't the full pkg import path, or begins with ./ - // Use path relative to current dir - // TODO: Turn abs path into ./... relative path as well - if !filepath.IsAbs(dir) && !strings.HasPrefix(dir, ".") { - dir = fmt.Sprintf(".%s%s", string(filepath.Separator), dir) - } - // TODO(hasbro17): AddDirRecursive can be noisy with klog warnings - // when it skips directories with no .go files. - // Silence those warnings unless in debug mode. - if err := p.AddDirRecursive(dir); err != nil { - return nil, err - } - universe, err := p.FindTypes() - if err != nil { - return nil, err - } - return universe, nil -} - -// getTypesForPkgPath find the pkg with the given path in universe -func getTypesForPkgPath(pkgPath string, universe types.Universe) (pkgTypes []*types.Type, err error) { - var pkg *types.Package - for _, upkg := range universe { - if strings.HasSuffix(upkg.Path, pkgPath) { - pkg = upkg - break - } - } - if pkg == nil { - return nil, fmt.Errorf("no package found for API %s", pkgPath) - } - for _, t := range pkg.Types { - pkgTypes = append(pkgTypes, t) - } - return pkgTypes, nil -} - -func getTypesForPkgName(pkgName string, universe types.Universe) (pkgTypes []*types.Type, err error) { - var pkg *types.Package - for _, upkg := range universe { - if upkg.Name == pkgName { - pkg = upkg - break - } - } - if pkg == nil { - return nil, fmt.Errorf("no package found for %s", pkgName) - } - for _, t := range pkg.Types { - pkgTypes = append(pkgTypes, t) - } - return pkgTypes, nil -} - -func findKindType(kind string, pkgTypes []*types.Type) *types.Type { - for _, t := range pkgTypes { - if t.Name.Name == kind { - return t - } - } - return nil -} diff --git a/internal/generate/olm-catalog/descriptor/descriptor_test.go b/internal/generate/olm-catalog/descriptor/descriptor_test.go deleted file mode 100644 index aa61905953..0000000000 --- a/internal/generate/olm-catalog/descriptor/descriptor_test.go +++ /dev/null @@ -1,298 +0,0 @@ -// Copyright 2019 The Operator-SDK Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package descriptor - -import ( - "os" - "path/filepath" - "reflect" - "strings" - "testing" - - "github.com/operator-framework/operator-sdk/internal/util/diffutil" - - olmapiv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1" - "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/gengo/types" - "sigs.k8s.io/yaml" -) - -var ( - testDataDir = filepath.Join("..", "..", "testdata", "go") -) - -func TestGetKindTypeForAPI(t *testing.T) { - multiAPIRootDir := filepath.Join("pkg", "apis") - singleAPIRootDir := "api" - group := "cache" - version := "v1alpha1" - - subTests := []struct { - description string - // path to apis types root dir e.g pkg/apis - apisDir string - // path to kind api pkg e.g pkg/apis/cache/v1alpha1 - expectedPkgPath string - group string - version string - kind string - numPkgTypes int - wantNil bool - // True if apis dir in the expected single or multi group layout - isExpectedLayout bool - }{ - { - "Must Succeed: Find types for Kind from multi APIs root directory", - multiAPIRootDir, - filepath.Join(multiAPIRootDir, group, version), - group, - version, - "Dummy", - 22, - false, - true, - }, - { - "Must Fail: Find types for non-existing Kind from multi APIs root directory", - multiAPIRootDir, - filepath.Join(multiAPIRootDir, group, version), - group, - version, - "NotFound", - 22, - true, - true, - }, - { - "Must Succeed: Find types for Kind from single APIs root directory", - singleAPIRootDir, - filepath.Join(singleAPIRootDir, version), - group, - version, - "Memcached", - 4, - false, - true, - }, - { - "Must Fail: Find types for non-existing Kind from single APIs root directory", - singleAPIRootDir, - filepath.Join(singleAPIRootDir, version), - group, - version, - "NotFound", - 4, - true, - true, - }, - // TODO: Add cases for non-standard api dir layouts: pkg/apis///version - } - - // Change directory to test data dir so the test cases can form the correct pkg imports - wd, err := os.Getwd() - if err != nil { - t.Fatal(err) - } - if err := os.Chdir(testDataDir); err != nil { - t.Fatal(err) - } - defer func() { - if err = os.Chdir(wd); err != nil { - t.Fatal(err) - } - }() - - for _, st := range subTests { - t.Run(st.description, func(t *testing.T) { - expectedPkgPath, err := getExpectedPkgLayout(st.apisDir, st.group, st.version) - if err != nil { - t.Fatalf("Failed to getExpectedPkgLayout(%s, %s, %s): %v", st.apisDir, st.group, st.version, err) - } - if st.isExpectedLayout { - if expectedPkgPath == "" || !strings.HasSuffix(expectedPkgPath, st.expectedPkgPath) { - t.Fatalf("Expected (%s) as suffix to expected pkg path (%s)", st.expectedPkgPath, expectedPkgPath) - } - } - - var pkgTypes []*types.Type - if st.isExpectedLayout { - universe, err := getPkgsFromDirRecursive(expectedPkgPath) - if err != nil { - t.Fatalf("Failed to get universe of types from API root directory (%s): %v)", st.apisDir, err) - } - pkgTypes, err = getTypesForPkgPath(expectedPkgPath, universe) - if err != nil { - t.Fatalf("Failed to get types of pkg path (%s) from API root directory(%s): %v)", - expectedPkgPath, st.apisDir, err) - } - } else { - universe, err := getPkgsFromDirRecursive(st.apisDir) - if err != nil { - t.Fatalf("Failed to get universe of types from API root directory (%s): %v)", st.apisDir, err) - } - pkgTypes, err = getTypesForPkgName(st.version, universe) - if err != nil { - t.Fatalf("Failed to get types of pkg name (%s) from API root directory(%s): %v)", st.version, st.apisDir, err) - } - } - - if n := len(pkgTypes); n != st.numPkgTypes { - t.Errorf("Expected %d package types, got %d", st.numPkgTypes, n) - } - kindType := findKindType(st.kind, pkgTypes) - if st.wantNil && kindType != nil { - t.Errorf("Expected type %q to not be found", kindType.Name) - } - if !st.wantNil && kindType == nil { - t.Errorf("Expected type %q to be found", st.kind) - } - if !st.wantNil && kindType != nil && kindType.Name.Name != st.kind { - t.Errorf("Expected type %q to have type name %q", kindType.Name, st.kind) - } - }) - } -} - -func TestGetCRDDescriptionForGVK(t *testing.T) { - - wd, err := os.Getwd() - if err != nil { - t.Fatal(err) - } - if err := os.Chdir(testDataDir); err != nil { - t.Fatal(err) - } - defer func() { - if err = os.Chdir(wd); err != nil { - t.Fatal(err) - } - }() - xdescsFor := func(paths ...string) (xdescs []string) { - for _, p := range paths { - xdescs = append(xdescs, getSpecXDescriptorsByPath(nil, p)...) - } - return xdescs - } - - // TODO(hasbro17): Change to run as subtests - cases := []struct { - description string - apisDir string - gvk schema.GroupVersionKind - expected olmapiv1alpha1.CRDDescription - wantErr bool - expErr error - }{ - { - "Populate CRDDescription successfully", - filepath.Join("pkg", "apis"), - schema.GroupVersionKind{Group: "cache.example.com", Version: "v1alpha1", Kind: "Dummy"}, - olmapiv1alpha1.CRDDescription{ - Kind: "Dummy", - Version: "v1alpha1", - DisplayName: "Dummy App", - Description: "Dummy is the Schema for the dummy API", - Resources: []olmapiv1alpha1.APIResourceReference{ - {Name: "dummy-deployment", Kind: "Deployment", Version: "v1"}, - {Name: "dummy-pod", Kind: "Pod", Version: "v1"}, - {Name: "dummy-replicaset", Kind: "ReplicaSet", Version: "v1beta2"}, - }, - SpecDescriptors: []olmapiv1alpha1.SpecDescriptor{ - {Path: "size", DisplayName: "dummy-pods", Description: "Should be in spec", - XDescriptors: xdescsFor("size")}, - {Path: "wheels", DisplayName: "Wheels", Description: "Should be in spec, but should not have array index in path", - XDescriptors: []string{"urn:alm:descriptor:com.tectonic.ui:text"}}, - {Path: "wheels[0].type", DisplayName: "Wheel Type", - Description: "Type should be in spec with path equal to wheels[0].type", - XDescriptors: []string{ - "urn:alm:descriptor:com.tectonic.ui:arrayFieldGroup:wheels", - "urn:alm:descriptor:com.tectonic.ui:text", - }}, - }, - StatusDescriptors: []olmapiv1alpha1.StatusDescriptor{ - {Path: "hog.engine", DisplayName: "boss-hog-engine", - Description: "Should be in status but not spec, since Hog isn't in DummySpec"}, - {Path: "hog.foo", DisplayName: "Public"}, - {Path: "hog.seatMaterial", DisplayName: "Seat Material"}, - {Path: "hog.seatMaterial", DisplayName: "Seat Material"}, - {Path: "nodes", DisplayName: "Nodes", - Description: "Should be in status but not spec, since DummyStatus isn't in DummySpec"}, - }, - }, - false, - nil, - }, - { - "Populate CRDDescription with non-standard spec type successfully", - filepath.Join("pkg", "apis"), - schema.GroupVersionKind{Group: "cache.example.com", Version: "v1alpha1", Kind: "OtherDummy"}, - olmapiv1alpha1.CRDDescription{ - Kind: "OtherDummy", - Version: "v1alpha1", - DisplayName: "Other Dummy App", - Description: "OtherDummy is the Schema for the other dummy API", - Resources: []olmapiv1alpha1.APIResourceReference{ - {Name: "other-dummy-pod", Kind: "Pod", Version: "v1"}, - {Name: "other-dummy-service", Kind: "Service", Version: "v1"}, - }, - SpecDescriptors: []olmapiv1alpha1.SpecDescriptor{ - {Path: "engine", DisplayName: "Engine", - Description: "Should be in status but not spec, since Hog isn't in DummySpec"}, - {Path: "foo", DisplayName: "Public"}, - {Path: "seatMaterial", DisplayName: "Seat Material"}, - {Path: "seatMaterial", DisplayName: "Seat Material"}, - }, - StatusDescriptors: []olmapiv1alpha1.StatusDescriptor{ - {Path: "nothing", DisplayName: "Nothing", - Description: "Should be in status but not spec, since this isn't a spec type"}, - }, - }, - false, - nil, - }, - { - "Fail to populate CRDDescription with skip on dir not exist", - filepath.Join("pkg", "notexist"), - schema.GroupVersionKind{Group: "cache.example.com", Version: "v1alpha1", Kind: "Dummy"}, - olmapiv1alpha1.CRDDescription{}, - true, - ErrAPIDirNotExist, - }, - { - "Fail to populate CRDDescription with skip on type", - filepath.Join("pkg", "apis"), - schema.GroupVersionKind{Group: "cache.example.com", Version: "v1alpha1", Kind: "NoKind"}, - olmapiv1alpha1.CRDDescription{}, - true, - ErrAPITypeNotFound, - }, - } - - for _, c := range cases { - description, err := GetCRDDescriptionForGVK(c.apisDir, c.gvk) - if !c.wantErr && err != nil { - t.Errorf("%s: expected nil error, got %q", c.description, err) - } else if c.wantErr && err == nil { - t.Errorf("%s: expected non-nil error, got nil error", c.description) - } else if !c.wantErr && err == nil { - if !reflect.DeepEqual(c.expected, description) { - be, _ := yaml.Marshal(c.expected) - bg, _ := yaml.Marshal(description) - t.Errorf("%s: populated CRDDescription not equal to expected:\n%s", - c.description, diffutil.Diff(string(be), string(bg))) - } - } - } -} diff --git a/internal/generate/olm-catalog/descriptor/parse.go b/internal/generate/olm-catalog/descriptor/parse.go deleted file mode 100644 index 695d4ddea3..0000000000 --- a/internal/generate/olm-catalog/descriptor/parse.go +++ /dev/null @@ -1,323 +0,0 @@ -// Copyright 2019 The Operator-SDK Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package descriptor - -import ( - "fmt" - "sort" - "strconv" - "strings" - "unicode" - - "github.com/operator-framework/operator-sdk/internal/annotations" - - "github.com/fatih/structtag" - olmapiv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1" - "k8s.io/gengo/types" -) - -const csvgenPrefix = annotations.SDKPrefix + ":gen-csv:" - -type descriptorType = string - -const ( - typeSpec descriptorType = "spec" - typeStatus descriptorType = "status" -) - -type descriptor struct { - // Use a SpecDescriptor since it has the same fields as a StatusDescriptor. - olmapiv1alpha1.SpecDescriptor - include bool - descType descriptorType -} - -func sortDescriptors(ds []descriptor) []descriptor { - sort.Slice(ds, func(i, j int) bool { - return ds[i].Path < ds[j].Path - }) - return ds -} - -type parsedCRDDescriptions struct { - descriptors []descriptor - displayName string - resources []olmapiv1alpha1.APIResourceReference -} - -func sortResources(rs []olmapiv1alpha1.APIResourceReference) []olmapiv1alpha1.APIResourceReference { - sort.Slice(rs, func(i, j int) bool { - return rs[i].Kind < rs[j].Kind - }) - return rs -} - -// parseCSVGenAnnotations parses all descriptor annotations from comments, -// each of which should contain one spec.customresourcedefinitions.owned entry. -// field Once all comments have been parsed, the entry is added to a -// parsedCRDDescriptions. -func parseCSVGenAnnotations(comments []string) (pd parsedCRDDescriptions, err error) { - tags := types.ExtractCommentTags(csvgenPrefix, comments) - specd, statusd := descriptor{descType: typeSpec}, descriptor{descType: typeStatus} - for path, vals := range tags { - pathElems, err := annotations.SplitPath(path) - if err != nil { - return parsedCRDDescriptions{}, err - } - parentPathElem, childPathElems := pathElems[0], pathElems[1:] - switch parentPathElem { - case "customresourcedefinitions": - switch childPathElems[0] { - case "specDescriptors": - err = parseMemberAnnotation(&specd, childPathElems, vals[0]) - if err != nil { - return parsedCRDDescriptions{}, err - } - case "statusDescriptors": - err = parseMemberAnnotation(&statusd, childPathElems, vals[0]) - if err != nil { - return parsedCRDDescriptions{}, err - } - case "displayName": - pd.displayName, err = strconv.Unquote(vals[0]) - if err != nil { - return parsedCRDDescriptions{}, fmt.Errorf("error unquoting displayName %s: %v", vals[0], err) - } - case "resources": - for _, v := range vals { - r, err := parseResource(v) - if err != nil { - return parsedCRDDescriptions{}, fmt.Errorf("error parsing resource %s: %v", v, err) - } - pd.resources = append(pd.resources, r) - } - default: - return parsedCRDDescriptions{}, fmt.Errorf("unsupported %s child path element %s", - parentPathElem, childPathElems[0]) - } - default: - return parsedCRDDescriptions{}, fmt.Errorf("unsupported path element %s", parentPathElem) - } - } - pd.descriptors = append(pd.descriptors, specd, statusd) - return pd, nil -} - -// parseMemberAnnotation determines which descriptor annotation was passed from -// pathElems and sets val to the corresponding field in d. -func parseMemberAnnotation(d *descriptor, pathElems []string, val string) (err error) { - switch len(pathElems) { - case 1: - // If this case is never entered, d will not be included. - d.include, err = strconv.ParseBool(val) - if err != nil { - return fmt.Errorf("error parsing %s bool val %s: %v", pathElems[0], val, err) - } - case 2: - switch pathElems[1] { - case "displayName": - d.DisplayName, err = strconv.Unquote(val) - if err != nil { - return fmt.Errorf("error unquoting field displayName %s: %v", val, err) - } - case "x-descriptors": - xdStr, err := strconv.Unquote(val) - if err != nil { - return fmt.Errorf("error unquoting field x-descriptors %s: %v", val, err) - } - d.XDescriptors = strings.Split(xdStr, ",") - default: - return fmt.Errorf("unsupported descriptor path element %s", pathElems[1]) - } - default: - return fmt.Errorf("unsupported descriptor path %s", annotations.JoinPath(pathElems...)) - } - return nil -} - -// parseResource parses a resource string of the form: -// "kind,version,\"quoted name\"" -func parseResource(rStr string) (r olmapiv1alpha1.APIResourceReference, err error) { - rStr, err = strconv.Unquote(rStr) - if err != nil { - return r, fmt.Errorf("error unquoting resource %s: %v", rStr, err) - } - rSplit := strings.SplitN(rStr, ",", 3) - if len(rSplit) < 2 { - return r, fmt.Errorf("resource string %s did not have at least a kind and a version", rStr) - } - r.Kind, r.Version = strings.TrimSpace(rSplit[0]), strings.TrimSpace(rSplit[1]) - if len(rSplit) == 3 { - r.Name, err = strconv.Unquote(rSplit[2]) - if err != nil { - return r, fmt.Errorf("error unquoting resource name %s: %v", rSplit[2], err) - } - r.Name = strings.TrimSpace(r.Name) - } - return r, nil -} - -// parseDescription joins comment strings into one line, removing any tool -// directives. -func parseDescription(comments []string) string { - var lines []string - for _, c := range comments { - l := strings.TrimSpace(strings.TrimLeft(c, "/")) - if l == "" || strings.HasPrefix(l, "+") { - continue - } - lines = append(lines, l) - } - return strings.Join(lines, " ") -} - -const ( - inlinedTag = "##inline##" - ignoredTag = "##ignore##" -) - -// getPathFromMember constructs a path from data in member, either from -// its struct tags or name. -func getPathFromMember(member types.Member) (string, error) { - // Embedded fields are inlined and children may be included. - if member.Embedded { - return inlinedTag, nil - } - // Unexported fields should be ignored in downstream processing. - if isNotExported(member.Name) { - return ignoredTag, nil - } - tags, err := structtag.Parse(member.Tags) - if err != nil { - return "", err - } - jsonTag, err := tags.Get("json") - if err == nil { - // Parse returns an error if no JSON tag is in tags, at which point we'll - // use another method to get path. - switch { - case contains(jsonTag.Options, "inline"): - return inlinedTag, nil - case jsonTag.Name == "-": - if len(jsonTag.Options) == 0 { - return ignoredTag, nil - } - return jsonTag.Name, nil - case jsonTag.Name != "": - return jsonTag.Name, nil - } - } - // There is no JSON tag in tags or tag name is empty. - // Use member name as path as json.Marshal does. - return member.Name, nil -} - -// isNotExported returns true if name is not an exported struct field name. -func isNotExported(name string) bool { - if len(name) == 0 { - return true - } - return unicode.IsLower(rune(name[0])) -} - -func contains(options []string, key string) bool { - for _, opt := range options { - if opt == key { - return true - } - } - return false -} - -func isPathInline(path string) bool { - return path == inlinedTag -} - -func isPathIgnore(path string) bool { - return path == ignoredTag -} - -// From https://github.com/openshift/console/blob/feabd61/frontend/packages/operator-lifecycle-manager/src/components/descriptors/types.ts#L3-L26 -var specXDescriptors = map[string]string{ - "size": "urn:alm:descriptor:com.tectonic.ui:podCount", - "podCount": "urn:alm:descriptor:com.tectonic.ui:podCount", - "endpoints": "urn:alm:descriptor:com.tectonic.ui:endpointList", - "endpointList": "urn:alm:descriptor:com.tectonic.ui:endpointList", - "label": "urn:alm:descriptor:com.tectonic.ui:label", - "resources": "urn:alm:descriptor:com.tectonic.ui:resourceRequirements", - "resourceRequirements": "urn:alm:descriptor:com.tectonic.ui:resourceRequirements", - "selector": "urn:alm:descriptor:com.tectonic.ui:selector:", - "namespaceSelector": "urn:alm:descriptor:com.tectonic.ui:namespaceSelector", - "booleanSwitch": "urn:alm:descriptor:com.tectonic.ui:booleanSwitch", - - "password": "urn:alm:descriptor:com.tectonic.ui:password", - "checkbox": "urn:alm:descriptor:com.tectonic.ui:checkbox", - "imagePullPolicy": "urn:alm:descriptor:com.tectonic.ui:imagePullPolicy", - "updateStrategy": "urn:alm:descriptor:com.tectonic.ui:updateStrategy", - "text": "urn:alm:descriptor:com.tectonic.ui:text", - "number": "urn:alm:descriptor:com.tectonic.ui:number", - "nodeAffinity": "urn:alm:descriptor:com.tectonic.ui:nodeAffinity", - "podAffinity": "urn:alm:descriptor:com.tectonic.ui:podAffinity", - "podAntiAffinity": "urn:alm:descriptor:com.tectonic.ui:podAntiAffinity", - "advanced": "urn:alm:descriptor:com.tectonic.ui:advanced", -} - -// getSpecXDescriptorsByPath uses path's elements to get x-descriptors a CRD -// descriptor should have. -func getSpecXDescriptorsByPath(existingXDescs []string, path string) []string { - return getXDescriptorsByPath(specXDescriptors, existingXDescs, path) -} - -// From https://github.com/openshift/console/blob/feabd61/frontend/packages/operator-lifecycle-manager/src/components/descriptors/types.ts#L28-L39 -var statusXDescriptors = map[string]string{ - "podStatuses": "urn:alm:descriptor:com.tectonic.ui:podStatuses", - "size": "urn:alm:descriptor:com.tectonic.ui:podCount", - "podCount": "urn:alm:descriptor:com.tectonic.ui:podCount", - "link": "urn:alm:descriptor:org.w3:link", - "w3link": "urn:alm:descriptor:org.w3:link", - "conditions": "urn:alm:descriptor:io.kubernetes.conditions", - "text": "urn:alm:descriptor:text", - "prometheusEndpoint": "urn:alm:descriptor:prometheusEndpoint", - "phase": "urn:alm:descriptor:io.kubernetes.phase", - "k8sPhase": "urn:alm:descriptor:io.kubernetes.phase", - "reason": "urn:alm:descriptor:io.kubernetes.phase:reason", - "k8sReason": "urn:alm:descriptor:io.kubernetes.phase:reason", -} - -// getStatusXDescriptorsByPath uses path's elements to get x-descriptors a CRD -// descriptor should have. -func getStatusXDescriptorsByPath(existingXDescs []string, path string) []string { - return getXDescriptorsByPath(statusXDescriptors, existingXDescs, path) -} - -func getXDescriptorsByPath(relevantXDescs map[string]string, existingXDescs []string, path string) (xdescs []string) { - // Ensure no duplicate x-descriptors are returned. - xdescMap := map[string]struct{}{} - for _, xd := range existingXDescs { - xdescMap[xd] = struct{}{} - } - pathSplit := strings.Split(path, ".") - for _, tag := range pathSplit { - xd, ok := relevantXDescs[tag] - if ok { - xdescMap[xd] = struct{}{} - } - } - for xd := range xdescMap { - xdescs = append(xdescs, xd) - } - sort.Strings(xdescs) - return xdescs -} diff --git a/internal/generate/olm-catalog/descriptor/parse_test.go b/internal/generate/olm-catalog/descriptor/parse_test.go deleted file mode 100644 index 1f47c016ff..0000000000 --- a/internal/generate/olm-catalog/descriptor/parse_test.go +++ /dev/null @@ -1,348 +0,0 @@ -// Copyright 2019 The Operator-SDK Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package descriptor - -import ( - "reflect" - "testing" - - "github.com/operator-framework/operator-sdk/internal/annotations" - "k8s.io/gengo/types" - - "github.com/operator-framework/api/pkg/operators/v1alpha1" -) - -func TestParseResource(t *testing.T) { - cases := []struct { - description string - input string - exp v1alpha1.APIResourceReference - wantErr bool - }{ - { - "Resource string no name", - `"Memcached,v1"`, - v1alpha1.APIResourceReference{Kind: "Memcached", Version: "v1"}, - false, - }, - { - "Resource string with name", - `"Memcached,v1,\"memcached.example.com\""`, - v1alpha1.APIResourceReference{Kind: "Memcached", Version: "v1", Name: "memcached.example.com"}, - false, - }, - { - "Resource string literal with name", - "`Memcached,v1,\"memcached.example.com\"`", - v1alpha1.APIResourceReference{Kind: "Memcached", Version: "v1", Name: "memcached.example.com"}, - false, - }, - { - "Empty resource string without quotes", - ``, v1alpha1.APIResourceReference{}, true, - }, - { - "Empty resource string with quotes", - `""`, v1alpha1.APIResourceReference{}, true, - }, - { - "Resource string with no version", - `"Memcached"`, v1alpha1.APIResourceReference{}, true, - }, - { - "Resource string with unquoted name", - `"Memcached,v1,memcached.example.com"`, v1alpha1.APIResourceReference{}, true, - }, - { - "Resource string literal with unquoted name", - "`Memcached,v1,memcached.example.com`", v1alpha1.APIResourceReference{}, true, - }, - } - - for _, c := range cases { - output, err := parseResource(c.input) - if err != nil { - if !c.wantErr { - t.Errorf("%s: expected nil error, got %q", c.description, err) - } - continue - } else if c.wantErr { - t.Errorf("%s: expected non-nil error, got nil error", c.description) - continue - } - - if !c.wantErr { - if c.exp != output { - t.Errorf("%s: expected %s, got %s", c.description, c.exp, output) - } - } - } -} - -func TestParseDescriptor(t *testing.T) { - cases := []struct { - description string - pathElems []string - val string - exp descriptor - wantErr bool - }{ - { - "Descriptor with true", - []string{"specDescriptors"}, "true", - descriptor{include: true}, - false, - }, - { - "Descriptor with false", - []string{"specDescriptors"}, "false", - descriptor{include: false}, - false, - }, - { - "Descriptor with displayName", - []string{"specDescriptors", "displayName"}, `"Some display name"`, - descriptor{SpecDescriptor: v1alpha1.SpecDescriptor{DisplayName: "Some display name"}}, - false, - }, - { - "Descriptor with x-descriptors", - []string{"specDescriptors", "x-descriptors"}, `"x:descriptor:ui:hint"`, - descriptor{SpecDescriptor: v1alpha1.SpecDescriptor{XDescriptors: []string{"x:descriptor:ui:hint"}}}, - false, - }, - { - "Descriptor with non-boolean", - []string{"specDescriptors"}, "foo", descriptor{}, - true, - }, - { - "Empty descriptor path elements", - []string{}, "", descriptor{}, - true, - }, - { - "Descriptor with too many path elements", - []string{"a", "b", "c"}, "", descriptor{}, - true, - }, - { - "Descriptor string with unknown path element", - []string{"specDescriptors", "bar"}, "", descriptor{}, - true, - }, - } - - for _, c := range cases { - output := descriptor{} - err := parseMemberAnnotation(&output, c.pathElems, c.val) - if err != nil { - if !c.wantErr { - t.Errorf("%s: expected nil error, got %q", c.description, err) - } - continue - } else if c.wantErr { - t.Errorf("%s: expected non-nil error, got nil error", c.description) - continue - } - - if !c.wantErr { - if !reflect.DeepEqual(c.exp, output) { - t.Errorf("%s: expected %v, got %v", c.description, c.exp, output) - } - } - } -} - -func TestParseCSVGenAnnotations(t *testing.T) { - crdPath := annotations.JoinPrefix(csvgenPrefix, "customresourcedefinitions") - specDescPath := annotations.JoinPath(crdPath, "specDescriptors") - statusDescPath := annotations.JoinPath(crdPath, "statusDescriptors") - displayNamePath := annotations.JoinPath(crdPath, "displayName") - resourcesPath := annotations.JoinPath(crdPath, "resources") - emptyDescriptors := []descriptor{{descType: typeSpec}, {descType: typeStatus}} - - cases := []struct { - description string - comments []string - exp parsedCRDDescriptions - wantErr bool - }{ - { - "Comment on type with one annotation", - []string{ - annotations.JoinAnnotation(displayNamePath, `"foo"`), - }, - parsedCRDDescriptions{displayName: "foo", descriptors: emptyDescriptors}, - false, - }, - { - "Comment on type with description and all annotations", - []string{ - annotations.JoinAnnotation(displayNamePath, `"foo"`), - annotations.JoinAnnotation(resourcesPath, `"Pod,v1,\"pod\""`), - annotations.JoinAnnotation(resourcesPath, `"Deployment,v1"`), - annotations.JoinAnnotation(resourcesPath, `"Service,v1,\"some.example.service.com\""`), - }, - parsedCRDDescriptions{ - displayName: "foo", - resources: []v1alpha1.APIResourceReference{ - {Kind: "Pod", Version: "v1", Name: "pod"}, - {Kind: "Deployment", Version: "v1"}, - {Kind: "Service", Version: "v1", Name: "some.example.service.com"}, - }, - descriptors: emptyDescriptors, - }, - false, - }, - { - "Comment on type member with spec inclusion annotation", - []string{ - annotations.JoinAnnotation(specDescPath, "true"), - }, - parsedCRDDescriptions{ - descriptors: []descriptor{{include: true, descType: typeSpec}, {descType: typeStatus}}, - }, - false, - }, - { - "Comment on type member with one spec annotation and no spec inclusion annotation", - []string{ - annotations.JoinAnnotation(annotations.JoinPath(specDescPath, "displayName"), `"foo"`), - }, - parsedCRDDescriptions{ - descriptors: []descriptor{ - {SpecDescriptor: v1alpha1.SpecDescriptor{DisplayName: "foo"}, descType: typeSpec}, - {include: false, descType: typeStatus}, - }, - }, - false, - }, - { - "Comment on type member with one spec and status annotation and no status inclusion annotation", - []string{ - annotations.JoinAnnotation(specDescPath, "true"), - annotations.JoinAnnotation(annotations.JoinPath(specDescPath, "displayName"), `"foo"`), - annotations.JoinAnnotation(annotations.JoinPath(statusDescPath, "displayName"), `"foo"`), - }, - parsedCRDDescriptions{ - descriptors: []descriptor{ - {include: true, descType: typeSpec, SpecDescriptor: v1alpha1.SpecDescriptor{DisplayName: "foo"}}, - {include: false, descType: typeStatus, SpecDescriptor: v1alpha1.SpecDescriptor{DisplayName: "foo"}}, - }, - }, - false, - }, - { - "Comment on type member with spec and status annotations and both inclusion annotations", - []string{ - annotations.JoinAnnotation(specDescPath, "true"), - annotations.JoinAnnotation(statusDescPath, "true"), - annotations.JoinAnnotation(annotations.JoinPath(specDescPath, "displayName"), `"foo"`), - annotations.JoinAnnotation(annotations.JoinPath(specDescPath, "x-descriptors"), `"some:ui:hint"`), - annotations.JoinAnnotation(annotations.JoinPath(statusDescPath, "displayName"), `"foo"`), - annotations.JoinAnnotation(annotations.JoinPath(statusDescPath, "x-descriptors"), - `"some:ui:hint"`), - }, - parsedCRDDescriptions{ - descriptors: []descriptor{ - {include: true, descType: typeSpec, SpecDescriptor: v1alpha1.SpecDescriptor{DisplayName: "foo", - XDescriptors: []string{"some:ui:hint"}}}, - {include: true, descType: typeStatus, SpecDescriptor: v1alpha1.SpecDescriptor{DisplayName: "foo", - XDescriptors: []string{"some:ui:hint"}}}, - }, - }, - false, - }, - { - "Comment on type with unknown path element", - []string{ - annotations.JoinAnnotation(annotations.JoinPath(annotations.JoinPrefix(csvgenPrefix, "unknown"), - "resources"), `"Deployment,v1"`), - }, - parsedCRDDescriptions{}, - true, - }, - { - "Comment on type with unknown child path element", - []string{ - annotations.JoinAnnotation(annotations.JoinPath(crdPath, "unknown"), `"Deployment,v1"`), - }, - parsedCRDDescriptions{}, - true, - }, - { - "Comment on type with a bad displayName annotation", - []string{ - annotations.JoinAnnotation(displayNamePath, `foo`), - }, - parsedCRDDescriptions{}, - true, - }, - } - - for _, c := range cases { - output, err := parseCSVGenAnnotations(c.comments) - if !c.wantErr && err != nil { - t.Errorf("%s: expected nil error, got %q", c.description, err) - } else if c.wantErr && err == nil { - t.Errorf("%s: expected non-nil error, got nil error", c.description) - } else if !c.wantErr && err == nil { - if !reflect.DeepEqual(c.exp, output) { - t.Errorf("%s:\nexpected\n\t%v\ngot\n\t%v", c.description, c.exp, output) - } - } - } -} - -func TestParsePathFromMember(t *testing.T) { - cases := []struct { - description string - member types.Member - exp string - wantErr bool - }{ - {"empty tag", types.Member{}, "Foo", false}, - {"valid single tag", types.Member{Tags: `json:"foo"`}, "foo", false}, - {"valid single with omitempty tag", types.Member{Tags: `json:"foo,omitempty"`}, - "foo", false}, - {"valid empty with omitempty tag", types.Member{Tags: `json:",omitempty"`}, - "Foo", false}, - {"valid single with inline tag", types.Member{Tags: `json:"foo,inline"`}, - inlinedTag, false}, - {"valid empty with inline tag", types.Member{Tags: `json:",inline"`}, - inlinedTag, false}, - {"valid ignore tag", types.Member{Tags: `json:"-"`}, ignoredTag, false}, - {"valid hyphen as name", types.Member{Tags: `json:"-,"`}, "-", false}, - {"JSON tag in multiple tags", types.Member{Tags: `json:"foo" protobuf:"bar"`}, - "foo", false}, - {"no JSON tag in tags", types.Member{Tags: `protobuf:"foo"`}, "Foo", false}, - {"invalid tags", types.Member{Tags: `blahblah`}, "", true}, - } - - for _, c := range cases { - c.member.Name = "Foo" - output, err := getPathFromMember(c.member) - if !c.wantErr && err != nil { - t.Errorf("%s: expected nil error, got %q", c.description, err) - } else if c.wantErr && err == nil { - t.Errorf("%s: expected non-nil error, got nil error", c.description) - } else if !c.wantErr && err == nil { - if c.exp != output { - t.Errorf("%s:\nexpected\n\t%v\ngot\n\t%v", c.description, c.exp, output) - } - } - } -} diff --git a/internal/generate/olm-catalog/descriptor/search.go b/internal/generate/olm-catalog/descriptor/search.go deleted file mode 100644 index 0924316399..0000000000 --- a/internal/generate/olm-catalog/descriptor/search.go +++ /dev/null @@ -1,149 +0,0 @@ -// Copyright 2019 The Operator-SDK Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package descriptor - -import ( - "fmt" - "strings" - - "github.com/operator-framework/operator-sdk/internal/util/k8sutil" - - "k8s.io/gengo/types" -) - -type typeTree interface { - getDescriptorsFor(descriptorType) ([]descriptor, error) -} - -type ttree struct { - root *types.Type - annotated []*tnode -} - -type tnode struct { - member types.Member - children []*tnode - pathSegments []string -} - -// newTypeTreeFromRoot collects all struct members in root and stores them in -// an ttree, along with any members that have annotations. -func newTypeTreeFromRoot(root *types.Type) (typeTree, error) { - tree := ttree{root: root} - nextChildren := []*tnode{{member: types.Member{Type: root}}} - lenNextChildren := len(nextChildren) - for len(nextChildren) > 0 { - for _, child := range nextChildren { - ct := getUnderlyingType(child.member.Type) - for _, cm := range ct.Members { - node := &tnode{member: cm} - // Parse path here so we can re-construct the path hierarchy later. - path, err := getPathFromMember(cm) - if err != nil { - return nil, fmt.Errorf("error parsing %s type member %s JSON tags: %v", child.member.Type.Name, cm.Name, err) - } - node.pathSegments = getPathSegments(child, path) - if hasAnnotations(cm) { - tree.annotated = append(tree.annotated, node) - } - child.children = append(child.children, node) - nextChildren = append(nextChildren, node) - } - } - nextChildren = nextChildren[lenNextChildren:] - lenNextChildren = len(nextChildren) - } - return &tree, nil -} - -// getDescriptorsFor returns descriptors for each annotated type in tree -// for a given descriptorType by parsing annotations on each type member. -func (tree *ttree) getDescriptorsFor(descType descriptorType) (descriptors []descriptor, err error) { - for _, node := range tree.annotated { - parsedDescriptors, err := parseCSVGenAnnotations(node.member.CommentLines) - if err != nil { - return nil, err - } - for _, d := range parsedDescriptors.descriptors { - if d.include && d.descType == descType { - pathBuilder := &strings.Builder{} - var hasIgnore, hasInline, includeInlined bool - lastIdx := len(node.pathSegments) - 1 - for segmentIdx, segment := range node.pathSegments { - // Ignored members are not serialized and therefore its own tag and - // all children should not be included in the final path. - if isPathIgnore(segment) { - hasIgnore = true - break - } - // Inlined members move their fields into the parent if the inlined - // member is not a leaf. This condition prevents inlined annotated - // members from having incorrect paths. - if isPathInline(segment) { - hasInline = true - includeInlined = segmentIdx < lastIdx || includeInlined - continue - } - pathBuilder.WriteString(segment) - pathBuilder.WriteString(".") - } - if hasIgnore || (hasInline && !includeInlined) { - continue - } - d.Path = strings.Trim(pathBuilder.String(), ".") - if d.DisplayName == "" { - d.DisplayName = k8sutil.GetDisplayName(node.member.Name) - } - d.Description = parseDescription(node.member.CommentLines) - switch d.descType { - case typeSpec: - d.XDescriptors = getSpecXDescriptorsByPath(d.XDescriptors, d.Path) - case typeStatus: - d.XDescriptors = getStatusXDescriptorsByPath(d.XDescriptors, d.Path) - } - descriptors = append(descriptors, d) - } - } - } - return sortDescriptors(descriptors), nil -} - -// getUnderlyingType returns either the Elem or Underlying type of t if t. -func getUnderlyingType(t *types.Type) *types.Type { - switch t.Kind { - case types.Map, types.Slice, types.Pointer, types.Chan: - t = t.Elem - case types.Alias, types.DeclarationOf: - t = t.Underlying - } - return t -} - -func getPathSegments(parent *tnode, path string) []string { - childPathSegments := make([]string, len(parent.pathSegments)+1) - copy(childPathSegments, parent.pathSegments) - - // If the parent is a slice, include an array - // index on the parent's path segment. - if parent.member.Type.Kind == types.Slice { - childPathSegments[len(childPathSegments)-2] += "[0]" - } - childPathSegments[len(childPathSegments)-1] = path - return childPathSegments -} - -func hasAnnotations(m types.Member) bool { - return len(types.ExtractCommentTags(csvgenPrefix, m.CommentLines)) != 0 -} diff --git a/website/content/en/docs/building-operators/golang/references/markers.md b/website/content/en/docs/building-operators/golang/references/markers.md index b3e4597862..b05a4924b7 100644 --- a/website/content/en/docs/building-operators/golang/references/markers.md +++ b/website/content/en/docs/building-operators/golang/references/markers.md @@ -148,7 +148,8 @@ These examples assume `Memcached`, `MemcachedSpec`, and `MemcachedStatus` are th ## Deprecated markers -_These markers are deprecated. You can migrate to the [new marker system](#clusterserviceversion-markers) by running the following script:_ +[Markers][deprecated-markers] supported by `operator-sdk` prior to v1.0.0 are deprecated. +You can migrate to the new marker system by running the following script: ```console $ curl -sSLo migrate-markers.sh https://raw.githubusercontent.com/operator-framework/operator-sdk/master/hack/generate/migrate-markers.sh @@ -156,33 +157,10 @@ $ chmod +x ./migrate-markers.sh $ ./migrate-markers.sh path/to/*_types.go ``` -All markers have a `+operator-sdk:gen-csv` prefix, denoting that they're parsed while executing -[`operator-sdk generate kustomize manifests`][cli-gen-kustomize-manifests]. - -### Paths - -Paths are dot-separated string hierarchies with the above prefix that map to CSV [`spec`][csv-spec] field names. - -Example: `+operator-sdk:gen-csv:customresourcedefinitions.specDescriptors.displayName="Pod Count"` - -#### customresourcedefinitions - -- `customresourcedefinitions`: child path token - - `displayName`: quoted string or string literal - - `resources`: quoted string or string literal, in the format `"kind,version,\"name\""` or `` `kind,version,"name"` ``, where `kind`, `version`, and `name` are fields in each CSV `resources` entry - - `specDescriptors`, `statusDescriptors`: bool, or child path token - - `displayName`: quoted string or string literal - - `x-descriptors`: quoted string or string literal comma-separated list of [`x-descriptor`][csv-x-desc] UI hints. - -**NOTES** -- `specDescriptors` and `statusDescriptors` with a value of `true` is required for each field to be included in their respective `customresourcedefinitions` CSV fields. See the examples below. -- `customresourcedefinitions` top-level `kind`, `name`, and `version` fields are parsed from API code. -- All `description` fields are parsed from type declaration and `struct` type field comments. -- `path` is parsed out of a field's JSON tag and merged with parent field path's in dot-hierarchy notation. - [markers]:https://pkg.go.dev/sigs.k8s.io/controller-tools/pkg/markers [cli-gen-kustomize-manifests]:/docs/cli/operator-sdk_generate_kustomize_manifests [csv-x-desc]:https://github.com/openshift/console/blob/master/frontend/packages/operator-lifecycle-manager/src/components/descriptors/reference/reference.md [csv-spec]:https://github.com/operator-framework/operator-lifecycle-manager/blob/e0eea22/doc/design/building-your-csv.md [csv-crds]:https://github.com/operator-framework/operator-lifecycle-manager/blob/master/doc/design/building-your-csv.md#your-custom-resource-definitions +[deprecated-markers]:https://v0-19-x.sdk.operatorframework.io/docs/golang/references/markers/ From 598997ba5f2903a173013b3bcf4b6fd51a01aa48 Mon Sep 17 00:00:00 2001 From: Eric Stroczynski Date: Wed, 22 Jul 2020 11:35:15 -0700 Subject: [PATCH 3/4] use GVKs directly instead of creating definition keys with potentially incorrect name --- .../bases/clusterserviceversion.go | 3 +- .../bases/definitions/crd.go | 35 ++++++-------- .../bases/definitions/definitions.go | 48 ++++++++++--------- .../bases/definitions/definitions_test.go | 22 ++++----- .../clusterserviceversion/bases/metadata.go | 20 -------- .../clusterserviceversion.go | 46 ------------------ 6 files changed, 53 insertions(+), 121 deletions(-) diff --git a/internal/generate/clusterserviceversion/bases/clusterserviceversion.go b/internal/generate/clusterserviceversion/bases/clusterserviceversion.go index 208726de10..7b420ce3d7 100644 --- a/internal/generate/clusterserviceversion/bases/clusterserviceversion.go +++ b/internal/generate/clusterserviceversion/bases/clusterserviceversion.go @@ -25,6 +25,7 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "sigs.k8s.io/yaml" + "github.com/operator-framework/operator-sdk/internal/generate/clusterserviceversion/bases/definitions" "github.com/operator-framework/operator-sdk/internal/util/k8sutil" "github.com/operator-framework/operator-sdk/internal/util/projutil" ) @@ -81,7 +82,7 @@ func (b ClusterServiceVersion) GetBase() (base *v1alpha1.ClusterServiceVersion, switch b.OperatorType { case projutil.OperatorTypeGo: // Update descriptions from the APIs dir. - err = updateDefinitions(base, b.APIsDir, b.GVKs) + err = definitions.ApplyDefinitionsForKeysGo(base, b.APIsDir, b.GVKs) } if err != nil { return nil, fmt.Errorf("error generating ClusterServiceVersion definitions metadata: %w", err) diff --git a/internal/generate/clusterserviceversion/bases/definitions/crd.go b/internal/generate/clusterserviceversion/bases/definitions/crd.go index 3933bcd516..8e134892bb 100644 --- a/internal/generate/clusterserviceversion/bases/definitions/crd.go +++ b/internal/generate/clusterserviceversion/bases/definitions/crd.go @@ -24,6 +24,7 @@ import ( "github.com/markbates/inflect" "github.com/operator-framework/api/pkg/operators/v1alpha1" "github.com/operator-framework/operator-registry/pkg/registry" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/version" crdmarkers "sigs.k8s.io/controller-tools/pkg/crd/markers" "sigs.k8s.io/controller-tools/pkg/markers" @@ -35,30 +36,20 @@ import ( func MakeKeyForCRDDescription(crd v1alpha1.CRDDescription) registry.DefinitionKey { return registry.DefinitionKey{ Name: crd.Name, - Group: MakeFullGroupForName(crd.Name), + Group: MakeFullGroupFromName(crd.Name), Version: crd.Version, Kind: crd.Kind, } } -// MakeCRDDescriptionForKey creates a crdDescription from a unique key. -func MakeCRDDescriptionForKey(key registry.DefinitionKey) v1alpha1.CRDDescription { - return v1alpha1.CRDDescription{ - Name: key.Name, - Version: key.Version, - Kind: key.Kind, - DisplayName: k8sutil.GetDisplayName(key.Kind), - } -} - -// MakeFullGroupForName returns everything but the first element of a CRD name, +// MakeFullGroupFromName returns everything but the first element of a CRD name, // which by definition is .. -func MakeFullGroupForName(name string) string { +func MakeFullGroupFromName(name string) string { return getHalfBySep(name, ".", 1) } -// MakeGroupForFullGroup returns the first element of an API group, ex. "foo" of "foo.example.com". -func MakeGroupForFullGroup(group string) string { +// MakeGroupFromFullGroup returns the first element of an API group, ex. "foo" of "foo.example.com". +func MakeGroupFromFullGroup(group string) string { return getHalfBySep(group, ".", 0) } @@ -73,11 +64,15 @@ func getHalfBySep(s, sep string, half uint) string { // buildCRDDescriptionFromType builds a crdDescription for the Go API defined // by key from markers and type information in g.types. -func (g generator) buildCRDDescriptionFromType(key registry.DefinitionKey, kindType *markers.TypeInfo) (v1alpha1.CRDDescription, error) { +func (g generator) buildCRDDescriptionFromType(gvk schema.GroupVersionKind, kindType *markers.TypeInfo) (v1alpha1.CRDDescription, error) { // Initialize the description. - description := MakeCRDDescriptionForKey(key) - description.Description = kindType.Doc + description := v1alpha1.CRDDescription{ + Description: kindType.Doc, + DisplayName: k8sutil.GetDisplayName(gvk.Kind), + Version: gvk.Version, + Kind: gvk.Kind, + } // Parse resources and displayName from the kind type's markers. for _, markers := range kindType.Markers { @@ -96,14 +91,14 @@ func (g generator) buildCRDDescriptionFromType(key registry.DefinitionKey, kindT } case crdmarkers.Resource: if d.Path != "" { - description.Name = fmt.Sprintf("%s.%s", d.Path, key.Group) + description.Name = fmt.Sprintf("%s.%s", d.Path, gvk.Group) } } } } // The default, if the resource marker's path value is not set, is to use a pluralized form of lowercase kind. if description.Name == "" { - description.Name = fmt.Sprintf("%s.%s", inflect.Pluralize(strings.ToLower(key.Kind)), key.Group) + description.Name = fmt.Sprintf("%s.%s", inflect.Pluralize(strings.ToLower(gvk.Kind)), gvk.Group) } sortDescription(description.Resources) diff --git a/internal/generate/clusterserviceversion/bases/definitions/definitions.go b/internal/generate/clusterserviceversion/bases/definitions/definitions.go index ff48f1153c..4d148db00f 100644 --- a/internal/generate/clusterserviceversion/bases/definitions/definitions.go +++ b/internal/generate/clusterserviceversion/bases/definitions/definitions.go @@ -20,9 +20,9 @@ import ( "path/filepath" "github.com/operator-framework/api/pkg/operators/v1alpha1" - "github.com/operator-framework/operator-registry/pkg/registry" log "github.com/sirupsen/logrus" "golang.org/x/tools/go/packages" + "k8s.io/apimachinery/pkg/runtime/schema" "sigs.k8s.io/controller-tools/pkg/genall" "sigs.k8s.io/controller-tools/pkg/loader" "sigs.k8s.io/controller-tools/pkg/markers" @@ -36,14 +36,13 @@ type descriptionValues struct { // ApplyDefinitionsForKeysGo collects markers and AST info on Go type declarations and struct fields // to populate csv spec fields. Go code with relevant markers and information is expected to be // in a package under apisRootDir and match a GVK in keys. -func ApplyDefinitionsForKeysGo(csv *v1alpha1.ClusterServiceVersion, apisRootDir string, - keys []registry.DefinitionKey) error { +func ApplyDefinitionsForKeysGo(csv *v1alpha1.ClusterServiceVersion, apisRootDir string, gvks []schema.GroupVersionKind) error { - // Construct a set of probable paths under apisRootDir for types defined by keys. + // Construct a set of probable paths under apisRootDir for types defined by gvks. // These are usually '(pkg/)?apis/(/)?'. // NB(estroz): using "leaf" packages prevents type builders from searching other packages. // It would be nice to implement extra-package traversal in the future. - paths, err := makeAPIPaths(apisRootDir, keys) + paths, err := makeAPIPaths(apisRootDir, gvks) if err != nil { return err } @@ -64,43 +63,43 @@ func ApplyDefinitionsForKeysGo(csv *v1alpha1.ClusterServiceVersion, apisRootDir } // Create definitions for kind types found under the collected roots. - definitionsByKey := make(map[registry.DefinitionKey]*descriptionValues) - for _, key := range keys { - kindType, hasKind := g.types[key.Kind] + definitionsByGVK := make(map[schema.GroupVersionKind]*descriptionValues) + for _, gvk := range gvks { + kindType, hasKind := g.types[gvk.Kind] if !hasKind { - log.Warnf("Skipping CSV annotation parsing for API %s: type %s not found", key, key.Kind) + log.Warnf("Skipping CSV annotation parsing for API %s: type %s not found", gvk, gvk.Kind) continue } - crd, err := g.buildCRDDescriptionFromType(key, kindType) + crd, err := g.buildCRDDescriptionFromType(gvk, kindType) if err != nil { return err } - definitionsByKey[key] = &descriptionValues{ + definitionsByGVK[gvk] = &descriptionValues{ crd: crd, } } // Update csv with all values parsed. - updateDefinitionsByKey(csv, definitionsByKey) + updateDefinitionsByKey(csv, definitionsByGVK) return nil } // makeAPIPaths creates a set of API directory paths with apisRootDir as their parent. -func makeAPIPaths(apisRootDir string, keys []registry.DefinitionKey) (paths []string, err error) { +func makeAPIPaths(apisRootDir string, gvks []schema.GroupVersionKind) (paths []string, err error) { if apisRootDir, err = filepath.Abs(apisRootDir); err != nil { return nil, err } - for _, key := range keys { + for _, gvk := range gvks { // Check if the kind pkg is at the expected layout. - group := MakeGroupForFullGroup(key.Group) - expectedPkgPath, err := getExpectedPkgLayout(apisRootDir, group, key.Version) + group := MakeGroupFromFullGroup(gvk.Group) + expectedPkgPath, err := getExpectedPkgLayout(apisRootDir, group, gvk.Version) if err != nil { return nil, err } if expectedPkgPath == "" { - log.Debugf("Skipping CSV annotation parsing for API %s: directory does not exist", key) + log.Warnf("Skipping CSV annotation parsing for API %s: directory does not exist", gvk) continue } paths = append(paths, expectedPkgPath) @@ -109,21 +108,24 @@ func makeAPIPaths(apisRootDir string, keys []registry.DefinitionKey) (paths []st } // updateDefinitionsByKey updates owned definitions that already exist in csv or adds new definitions that do not. -func updateDefinitionsByKey(csv *v1alpha1.ClusterServiceVersion, - definitionsByKey map[registry.DefinitionKey]*descriptionValues) { +func updateDefinitionsByKey(csv *v1alpha1.ClusterServiceVersion, defsByGVK map[schema.GroupVersionKind]*descriptionValues) { // Overwrite crdDescriptions we've parsed from Go source. for i := 0; i < len(csv.Spec.CustomResourceDefinitions.Owned); i++ { crd := csv.Spec.CustomResourceDefinitions.Owned[i] - key := MakeKeyForCRDDescription(crd) - if values, hasKey := definitionsByKey[key]; hasKey { + gvk := schema.GroupVersionKind{ + Group: MakeFullGroupFromName(crd.Name), + Version: crd.Version, + Kind: crd.Kind, + } + if values, hasKey := defsByGVK[gvk]; hasKey { csv.Spec.CustomResourceDefinitions.Owned[i] = values.crd - delete(definitionsByKey, key) + delete(defsByGVK, gvk) } } // Add any new crdDescriptions to the CSV. - for _, values := range definitionsByKey { + for _, values := range defsByGVK { csv.Spec.CustomResourceDefinitions.Owned = append(csv.Spec.CustomResourceDefinitions.Owned, values.crd) } } diff --git a/internal/generate/clusterserviceversion/bases/definitions/definitions_test.go b/internal/generate/clusterserviceversion/bases/definitions/definitions_test.go index 7064a928ea..e5fda6fdda 100644 --- a/internal/generate/clusterserviceversion/bases/definitions/definitions_test.go +++ b/internal/generate/clusterserviceversion/bases/definitions/definitions_test.go @@ -20,8 +20,8 @@ import ( "testing" "github.com/operator-framework/api/pkg/operators/v1alpha1" - "github.com/operator-framework/operator-registry/pkg/registry" "github.com/stretchr/testify/assert" + "k8s.io/apimachinery/pkg/runtime/schema" ) // TODO(estroz): migrate to ginkgo/gomega @@ -50,7 +50,7 @@ func TestApplyDefinitionsForKeysGo(t *testing.T) { description string apisDir string csv *v1alpha1.ClusterServiceVersion - keys []registry.DefinitionKey + gvks []schema.GroupVersionKind expectedCRDs v1alpha1.CustomResourceDefinitions wantErr bool }{ @@ -58,8 +58,8 @@ func TestApplyDefinitionsForKeysGo(t *testing.T) { description: "Populate CRDDescription successfully", apisDir: "api", csv: &v1alpha1.ClusterServiceVersion{}, - keys: []registry.DefinitionKey{ - {Name: "dummys.cache.example.com", Group: "cache.example.com", Version: "v1alpha2", Kind: "Dummy"}, + gvks: []schema.GroupVersionKind{ + {Group: "cache.example.com", Version: "v1alpha2", Kind: "Dummy"}, }, expectedCRDs: v1alpha1.CustomResourceDefinitions{ Owned: []v1alpha1.CRDDescription{ @@ -104,8 +104,8 @@ func TestApplyDefinitionsForKeysGo(t *testing.T) { description: "Populate CRDDescription with non-standard spec type successfully", apisDir: "api", csv: &v1alpha1.ClusterServiceVersion{}, - keys: []registry.DefinitionKey{ - {Name: "otherdummies.cache.example.com", Group: "cache.example.com", Version: "v1alpha2", Kind: "OtherDummy"}, + gvks: []schema.GroupVersionKind{ + {Group: "cache.example.com", Version: "v1alpha2", Kind: "OtherDummy"}, }, expectedCRDs: v1alpha1.CustomResourceDefinitions{ Owned: []v1alpha1.CRDDescription{ @@ -161,8 +161,8 @@ func TestApplyDefinitionsForKeysGo(t *testing.T) { }, }, }, - keys: []registry.DefinitionKey{ - {Name: "dummys.cache.example.com", Group: "cache.example.com", Version: "v1alpha2", Kind: "Dummy"}, + gvks: []schema.GroupVersionKind{ + {Group: "cache.example.com", Version: "v1alpha2", Kind: "Dummy"}, }, expectedCRDs: v1alpha1.CustomResourceDefinitions{ Owned: []v1alpha1.CRDDescription{ @@ -212,8 +212,8 @@ func TestApplyDefinitionsForKeysGo(t *testing.T) { }, }, }, - keys: []registry.DefinitionKey{ - {Name: "nokinds.cache.example.com", Group: "cache.example.com", Version: "v1alpha2", Kind: "NoKind"}, + gvks: []schema.GroupVersionKind{ + {Group: "cache.example.com", Version: "v1alpha2", Kind: "NoKind"}, }, expectedCRDs: v1alpha1.CustomResourceDefinitions{ Owned: []v1alpha1.CRDDescription{ @@ -240,7 +240,7 @@ func TestApplyDefinitionsForKeysGo(t *testing.T) { for _, c := range cases { t.Run(c.description, func(t *testing.T) { - err := ApplyDefinitionsForKeysGo(c.csv, c.apisDir, c.keys) + err := ApplyDefinitionsForKeysGo(c.csv, c.apisDir, c.gvks) if !c.wantErr && err != nil { t.Errorf("Expected nil error, got %q", err) } else if c.wantErr && err == nil { diff --git a/internal/generate/clusterserviceversion/bases/metadata.go b/internal/generate/clusterserviceversion/bases/metadata.go index 1015153550..57917c3ee0 100644 --- a/internal/generate/clusterserviceversion/bases/metadata.go +++ b/internal/generate/clusterserviceversion/bases/metadata.go @@ -15,15 +15,10 @@ package bases import ( - "fmt" "strings" - "github.com/markbates/inflect" "github.com/operator-framework/api/pkg/operators/v1alpha1" - "github.com/operator-framework/operator-registry/pkg/registry" - "k8s.io/apimachinery/pkg/runtime/schema" - "github.com/operator-framework/operator-sdk/internal/generate/clusterserviceversion/bases/definitions" "github.com/operator-framework/operator-sdk/internal/util/projutil" ) @@ -93,18 +88,3 @@ func (s uiMetadata) apply(csv *v1alpha1.ClusterServiceVersion) { csv.Spec.Provider = provider } } - -// updateDefinitions parses APIs in apisDir for code and markers that can build a crdDescription and -// updates existing crdDescriptions in csv. If no code/markers are found, the crdDescription is appended as-is. -func updateDefinitions(csv *v1alpha1.ClusterServiceVersion, apisDir string, gvks []schema.GroupVersionKind) error { - keys := make([]registry.DefinitionKey, len(gvks)) - for i, gvk := range gvks { - keys[i] = registry.DefinitionKey{ - Name: fmt.Sprintf("%s.%s", inflect.Pluralize(strings.ToLower(gvk.Kind)), gvk.Group), - Group: gvk.Group, - Version: gvk.Version, - Kind: gvk.Kind, - } - } - return definitions.ApplyDefinitionsForKeysGo(csv, apisDir, keys) -} diff --git a/internal/generate/clusterserviceversion/clusterserviceversion.go b/internal/generate/clusterserviceversion/clusterserviceversion.go index 505d9d46ab..686e301ea4 100644 --- a/internal/generate/clusterserviceversion/clusterserviceversion.go +++ b/internal/generate/clusterserviceversion/clusterserviceversion.go @@ -31,7 +31,6 @@ import ( "github.com/operator-framework/operator-sdk/internal/generate/clusterserviceversion/bases" "github.com/operator-framework/operator-sdk/internal/generate/collector" genutil "github.com/operator-framework/operator-sdk/internal/generate/internal" - "github.com/operator-framework/operator-sdk/internal/util/k8sutil" "github.com/operator-framework/operator-sdk/internal/util/projutil" ) @@ -247,51 +246,6 @@ func (g Generator) makeBaseGetter(basePath, apisDir string, interactive bool) ge } } -// makeBundleBaseGetterLegacy returns a function that gets a bundle base -// for legacy project layouts. -func (g Generator) makeBundleBaseGetterLegacy(inputDir, apisDir string, ilvl projutil.InteractiveLevel) getBaseFunc { - basePath := filepath.Join(inputDir, bundle.ManifestsDir, makeCSVFileName(g.OperatorName)) - if genutil.IsNotExist(basePath) { - basePath = "" - } - return g.makeBaseGetterLegacy(basePath, apisDir, requiresInteraction(basePath, ilvl)) -} - -// makePackageBaseGetterLegacy returns a function that gets a package base -// for legacy project layouts. -func (g Generator) makePackageBaseGetterLegacy(inputDir, apisDir string, ilvl projutil.InteractiveLevel) getBaseFunc { - basePath := filepath.Join(inputDir, g.Version, makeCSVFileName(g.OperatorName)) - if genutil.IsNotExist(basePath) { - basePath = "" - } - return g.makeBaseGetterLegacy(basePath, apisDir, requiresInteraction(basePath, ilvl)) -} - -// makeBaseGetterLegacy returns a function that gets a base from inputDir. -// apisDir is used by getBaseFunc to populate base fields. This method should -// be used when creating LegacyOptions. -func (g Generator) makeBaseGetterLegacy(basePath, apisDir string, interactive bool) getBaseFunc { - var gvks []schema.GroupVersionKind - if g.Collector != nil { - v1crdGVKs := k8sutil.GVKsForV1CustomResourceDefinitions(g.Collector.V1CustomResourceDefinitions...) - gvks = append(gvks, v1crdGVKs...) - v1beta1crdGVKs := k8sutil.GVKsForV1beta1CustomResourceDefinitions(g.Collector.V1beta1CustomResourceDefinitions...) - gvks = append(gvks, v1beta1crdGVKs...) - } - - return func() (*operatorsv1alpha1.ClusterServiceVersion, error) { - b := bases.ClusterServiceVersion{ - OperatorName: g.OperatorName, - OperatorType: g.OperatorType, - BasePath: basePath, - APIsDir: apisDir, - GVKs: gvks, - Interactive: interactive, - } - return b.GetBase() - } -} - // requiresInteraction checks if the combination of ilvl and basePath existence // requires the generator prompt a user interactively. func requiresInteraction(basePath string, ilvl projutil.InteractiveLevel) bool { From 6ee709277848a4ea7fbedd32c37e265c0fe1415e Mon Sep 17 00:00:00 2001 From: Eric Stroczynski Date: Wed, 22 Jul 2020 12:05:07 -0700 Subject: [PATCH 4/4] remove more unused code --- .../clusterserviceversion/bases/definitions/crd.go | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/internal/generate/clusterserviceversion/bases/definitions/crd.go b/internal/generate/clusterserviceversion/bases/definitions/crd.go index 8e134892bb..267d28bb11 100644 --- a/internal/generate/clusterserviceversion/bases/definitions/crd.go +++ b/internal/generate/clusterserviceversion/bases/definitions/crd.go @@ -23,7 +23,6 @@ import ( "github.com/fatih/structtag" "github.com/markbates/inflect" "github.com/operator-framework/api/pkg/operators/v1alpha1" - "github.com/operator-framework/operator-registry/pkg/registry" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/version" crdmarkers "sigs.k8s.io/controller-tools/pkg/crd/markers" @@ -32,16 +31,6 @@ import ( "github.com/operator-framework/operator-sdk/internal/util/k8sutil" ) -// MakeKeyForCRDDescription creates a key unique to a crdDescription. -func MakeKeyForCRDDescription(crd v1alpha1.CRDDescription) registry.DefinitionKey { - return registry.DefinitionKey{ - Name: crd.Name, - Group: MakeFullGroupFromName(crd.Name), - Version: crd.Version, - Kind: crd.Kind, - } -} - // MakeFullGroupFromName returns everything but the first element of a CRD name, // which by definition is .. func MakeFullGroupFromName(name string) string {