Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

internal/scaffold/crd.go: use structural schema for non-go operator CRDs #2275

Merged
merged 2 commits into from
Nov 28, 2019
Merged

internal/scaffold/crd.go: use structural schema for non-go operator CRDs #2275

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
41e6816
internal/scaffold/crd.go: use structural schema for non-go operator CRDs
joelanford Nov 27, 2019
9081bf6
CHANGELOG.md: update for #2275
joelanford Nov 27, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
- Updated `pkg/test/e2eutil.WaitForDeployment()` and `pkg/test/e2eutil.WaitForOperatorDeployment()` to successfully complete waiting when the available replica count is _at least_ (rather than exactly) the minimum replica count required. ([#2248](https://github.com/operator-framework/operator-sdk/pull/2248))
- Replace in the Ansible based operators module tests `k8s_info` for `k8s_facts` which is deprecated. ([#2168](https://github.com/operator-framework/operator-sdk/issues/2168))
- Upgrade the Ansible version from `2.8` to `2.9` on the Ansible based operators image. ([#2168](https://github.com/operator-framework/operator-sdk/issues/2168))
- Updated CRD generation for non-Go operators to use valid structural schema. ([#2275](https://github.com/operator-framework/operator-sdk/issues/2275))

### Deprecated

Expand Down
12 changes: 12 additions & 0 deletions internal/scaffold/crd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package scaffold

import (
goerrors "errors"
Copy link
Contributor

Choose a reason for hiding this comment

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

why create an alias ? Could not it jus be :

Suggested change
goerrors "errors"
"errors"

Copy link
Member Author

Choose a reason for hiding this comment

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

We're already using github.com/pkg/errors in this file, so one of them needs an alias. Rather than changing all of the existing usages, I thought it would be simpler and easier to review to use an alias here.

"fmt"
"io"
"os"
Expand Down Expand Up @@ -147,6 +148,10 @@ func (s *CRD) CustomRender() ([]byte, error) {
return nil, err
}
}
} else if goerrors.Is(err, afero.ErrFileNotFound) {
crd = newCRDForResource(s.Resource)
} else {
return nil, err
}
}

Expand Down Expand Up @@ -187,6 +192,7 @@ func runCRDGenerator(rule genall.OutputRule, root string) (err error) {
}

func newCRDForResource(r *Resource) *apiextv1beta1.CustomResourceDefinition {
trueVal := true
crd := &apiextv1beta1.CustomResourceDefinition{
TypeMeta: metav1.TypeMeta{
APIVersion: apiextv1beta1.SchemeGroupVersion.String(),
Expand All @@ -204,6 +210,12 @@ func newCRDForResource(r *Resource) *apiextv1beta1.CustomResourceDefinition {
Subresources: &apiextv1beta1.CustomResourceSubresources{
Status: &apiextv1beta1.CustomResourceSubresourceStatus{},
},
Validation: &apiextv1beta1.CustomResourceValidation{
OpenAPIV3Schema: &apiextv1beta1.JSONSchemaProps{
Type: "object",
XPreserveUnknownFields: &trueVal,
},
},
},
}
setCRDNamesForResource(crd, r)
Expand Down
60 changes: 54 additions & 6 deletions internal/scaffold/crd_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"path/filepath"
"testing"

"github.com/operator-framework/operator-sdk/internal/scaffold/input"
testutil "github.com/operator-framework/operator-sdk/internal/scaffold/internal/testutil"
"github.com/operator-framework/operator-sdk/internal/util/diffutil"
"github.com/operator-framework/operator-sdk/internal/util/fileutil"
Expand Down Expand Up @@ -122,7 +123,54 @@ spec:
storage: true
`

func TestCRDNonGoProject(t *testing.T) {
func TestCRDNonGoProjectDefault(t *testing.T) {
s, buf := setupScaffoldAndWriter()
s.Fs = afero.NewMemMapFs()

r, err := NewResource(appApiVersion, appKind)
if err != nil {
t.Fatal(err)
}

crd := &CRD{Resource: r}
cfg := &input.Config{}
if err = s.Execute(cfg, crd); err != nil {
t.Fatalf("Failed to execute the scaffold: (%v)", err)
}

if crdNonGoDefaultExp != buf.String() {
diffs := diffutil.Diff(crdNonGoDefaultExp, buf.String())
t.Fatalf("Expected vs actual differs.\n%v", diffs)
}
}

// crdNonGoDefaultExp is the default non-go CRD. Non-go projects don't have the
// luxury of kubebuilder annotations.
const crdNonGoDefaultExp = `apiVersion: apiextensions.k8s.io/v1beta1
kind: CustomResourceDefinition
metadata:
name: appservices.app.example.com
spec:
group: app.example.com
names:
kind: AppService
listKind: AppServiceList
plural: appservices
singular: appservice
scope: Namespaced
subresources:
status: {}
validation:
openAPIV3Schema:
type: object
x-kubernetes-preserve-unknown-fields: true
versions:
- name: v1alpha1
served: true
storage: true
`

func TestCRDNonGoProjectCustom(t *testing.T) {
s, buf := setupScaffoldAndWriter()
s.Fs = afero.NewMemMapFs()

Expand All @@ -142,7 +190,7 @@ func TestCRDNonGoProject(t *testing.T) {
}

path := filepath.Join(cfg.AbsProjectPath, i.Path)
err = afero.WriteFile(s.Fs, path, []byte(crdNonGoExp), fileutil.DefaultFileMode)
err = afero.WriteFile(s.Fs, path, []byte(crdNonGoCustomExp), fileutil.DefaultFileMode)
if err != nil {
t.Fatal(err)
}
Expand All @@ -151,16 +199,16 @@ func TestCRDNonGoProject(t *testing.T) {
t.Fatalf("Failed to execute the scaffold: (%v)", err)
}

if crdNonGoExp != buf.String() {
diffs := diffutil.Diff(crdNonGoExp, buf.String())
if crdNonGoCustomExp != buf.String() {
diffs := diffutil.Diff(crdNonGoCustomExp, buf.String())
t.Fatalf("Expected vs actual differs.\n%v", diffs)
}
}

// crdNonGoExp contains a simple validation block to make sure manually-added
// crdNonGoCustomExp contains a simple validation block to make sure manually-added
// validation is not overwritten. Non-go projects don't have the luxury of
// kubebuilder annotations.
const crdNonGoExp = `apiVersion: apiextensions.k8s.io/v1beta1
const crdNonGoCustomExp = `apiVersion: apiextensions.k8s.io/v1beta1
kind: CustomResourceDefinition
metadata:
name: appservices.app.example.com
Expand Down