Skip to content

Commit

Permalink
CRDs should not have status field. (kubeflow#1175)
Browse files Browse the repository at this point in the history
* CRDs should not have status field.

* kubeflow/manifests#1174 lots of CRDs have status fields which
  causes ACM to complain that the CRD is invalid.

* This PR cleans up those CRDs and adds an appropriate validation test.

* Fix typos in AWS application specs (kubeflow/testing#668)

* Update tests.
  • Loading branch information
jlewi authored May 18, 2020
1 parent e3eee68 commit 9e38132
Show file tree
Hide file tree
Showing 26 changed files with 79 additions and 173 deletions.
6 changes: 0 additions & 6 deletions application/application-crds/base/crd.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -231,9 +231,3 @@ spec:
type: integer
type: object
version: v1beta1
status:
acceptedNames:
kind: ""
plural: ""
conditions: []
storedVersions: []
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@ spec:
version: v0.1
description: Authorization adpator to append header for AWS application load balancer
maintainers:
- name: Jiaxin Shan
email: shjiaxin@amazon.com
owners:
- name: Jiaxin Shan
email: shjiaxin@amazon.com
- name: Jiaxin Shan
email: shjiaxin@amazon.com
owners:
- name: Jiaxin Shan
email: shjiaxin@amazon.com
keywords:
- aws
- istio
Expand Down
10 changes: 5 additions & 5 deletions aws/nvidia-device-plugin/overlays/application/application.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@ spec:
version: v1.0.0-beta
description: Nvidia Device Plugin for GPU nodes
maintainers:
- name: Jiaxin Shan
email: shjiaxin@amazon.com
owners:
- name: Jiaxin Shan
email: shjiaxin@amazon.com
- name: Jiaxin Shan
email: shjiaxin@amazon.com
owners:
- name: Jiaxin Shan
email: shjiaxin@amazon.com
keywords:
- aws
- nvidia
Expand Down
30 changes: 0 additions & 30 deletions cert-manager/cert-manager-crds/base/crd.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1367,12 +1367,6 @@ spec:
- name: v1alpha2
served: true
storage: true
status:
acceptedNames:
kind: ""
plural: ""
conditions: []
storedVersions: []

---
apiVersion: apiextensions.k8s.io/v1beta1
Expand Down Expand Up @@ -1575,12 +1569,6 @@ spec:
- name: v1alpha2
served: true
storage: true
status:
acceptedNames:
kind: ""
plural: ""
conditions: []
storedVersions: []

---
apiVersion: apiextensions.k8s.io/v1beta1
Expand Down Expand Up @@ -1764,12 +1752,6 @@ spec:
- name: v1alpha2
served: true
storage: true
status:
acceptedNames:
kind: ""
plural: ""
conditions: []
storedVersions: []

---

Expand Down Expand Up @@ -2008,12 +1990,6 @@ spec:
- name: v1alpha2
served: true
storage: true
status:
acceptedNames:
kind: ""
plural: ""
conditions: []
storedVersions: []

---

Expand Down Expand Up @@ -5336,9 +5312,3 @@ spec:
- name: v1alpha2
served: true
storage: true
status:
acceptedNames:
kind: ""
plural: ""
conditions: []
storedVersions: []
7 changes: 0 additions & 7 deletions jupyter/notebook-controller/base/crd.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,3 @@ spec:
required:
- conditions
type: object
status:
acceptedNames:
kind: ""
plural: ""
conditions: []
storedVersions: []

6 changes: 0 additions & 6 deletions kfserving/kfserving-crds/base/crd.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -605,9 +605,3 @@ spec:
type: string
type: object
version: v1alpha2
status:
acceptedNames:
kind: ""
plural: ""
conditions: []
storedVersions: []
6 changes: 0 additions & 6 deletions profiles/base/crd.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -154,9 +154,3 @@ spec:
- name: v1beta1
served: true
storage: false
status:
acceptedNames:
kind: ""
plural: ""
conditions: []
storedVersions: []
Original file line number Diff line number Diff line change
Expand Up @@ -2544,9 +2544,3 @@ spec:
- name: v1beta2
served: true
storage: true
status:
acceptedNames:
kind: ""
plural: ""
conditions: []
storedVersions: []
Original file line number Diff line number Diff line change
Expand Up @@ -2526,9 +2526,3 @@ spec:
- name: v1beta2
served: true
storage: true
status:
acceptedNames:
kind: ""
plural: ""
conditions: []
storedVersions: []
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,3 @@ spec:
- name: v1
served: true
storage: false
status:
acceptedNames:
kind: ""
plural: ""
conditions: []
storedVersions: []
Original file line number Diff line number Diff line change
Expand Up @@ -156,9 +156,3 @@ spec:
- name: v1beta1
served: true
storage: false
status:
acceptedNames:
kind: ""
plural: ""
conditions: []
storedVersions: []
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,3 @@ spec:
- name: v1
served: true
storage: false
status:
acceptedNames:
kind: ""
plural: ""
conditions: []
storedVersions: []
Original file line number Diff line number Diff line change
Expand Up @@ -156,9 +156,3 @@ spec:
- name: v1beta1
served: true
storage: false
status:
acceptedNames:
kind: ""
plural: ""
conditions: []
storedVersions: []
Original file line number Diff line number Diff line change
Expand Up @@ -231,9 +231,3 @@ spec:
type: integer
type: object
version: v1beta1
status:
acceptedNames:
kind: ""
plural: ""
conditions: []
storedVersions: []
Original file line number Diff line number Diff line change
Expand Up @@ -179,9 +179,3 @@ spec:
- name: v1alpha2
served: true
storage: true
status:
acceptedNames:
kind: ""
plural: ""
conditions: []
storedVersions: []
Original file line number Diff line number Diff line change
Expand Up @@ -233,9 +233,3 @@ spec:
- name: v1alpha2
served: true
storage: true
status:
acceptedNames:
kind: ""
plural: ""
conditions: []
storedVersions: []
Original file line number Diff line number Diff line change
Expand Up @@ -1367,9 +1367,3 @@ spec:
- name: v1alpha2
served: true
storage: true
status:
acceptedNames:
kind: ""
plural: ""
conditions: []
storedVersions: []
Original file line number Diff line number Diff line change
Expand Up @@ -1653,9 +1653,3 @@ spec:
- name: v1alpha2
served: true
storage: true
status:
acceptedNames:
kind: ""
plural: ""
conditions: []
storedVersions: []
Original file line number Diff line number Diff line change
Expand Up @@ -198,9 +198,3 @@ spec:
- name: v1alpha2
served: true
storage: true
status:
acceptedNames:
kind: ""
plural: ""
conditions: []
storedVersions: []
Original file line number Diff line number Diff line change
Expand Up @@ -611,9 +611,3 @@ spec:
type: string
type: object
version: v1alpha2
status:
acceptedNames:
kind: ""
plural: ""
conditions: []
storedVersions: []
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,3 @@ spec:
- name: v1
served: true
storage: false
status:
acceptedNames:
kind: ""
plural: ""
conditions: []
storedVersions: []
Original file line number Diff line number Diff line change
Expand Up @@ -162,9 +162,3 @@ spec:
- name: v1beta1
served: true
storage: false
status:
acceptedNames:
kind: ""
plural: ""
conditions: []
storedVersions: []
Original file line number Diff line number Diff line change
Expand Up @@ -2552,9 +2552,3 @@ spec:
- name: v1beta2
served: true
storage: true
status:
acceptedNames:
kind: ""
plural: ""
conditions: []
storedVersions: []
Original file line number Diff line number Diff line change
Expand Up @@ -2534,9 +2534,3 @@ spec:
- name: v1beta2
served: true
storage: true
status:
acceptedNames:
kind: ""
plural: ""
conditions: []
storedVersions: []
69 changes: 69 additions & 0 deletions tests/validate_resources_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@ package tests
import (
"github.com/ghodss/yaml"
"io/ioutil"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"os"
"path/filepath"
"regexp"
"sigs.k8s.io/kustomize/v3/pkg/types"
"testing"
)
Expand Down Expand Up @@ -91,3 +93,70 @@ func TestCommonLabelsImmutable(t *testing.T) {

}
}

// TestNoStatus is a test to try to ensure we don't include status in resources
// as this causes validation issues: https://github.com/kubeflow/manifests/issues/1174
func TestNoStatus(t *testing.T) {
rootDir := ".."

// Directories to exclude. Thee paths should be relative to rootDir.
// Subdirectories won't be searched
excludes := map[string]bool{
"tests": true,
".git": true,
".github": true,
"profiles/overlays/test": true,
// Skip cnrm-install. We don't install this with ACM so we don't need to fix it.
// It seems like if this is an issue it should eventually get fixed in upstream cnrm configs.
"gcp/v2/management/cnrm-install": true,
}

err := filepath.Walk("..", func(path string, info os.FileInfo, err error) error {
relPath, err := filepath.Rel(rootDir, path)

if err != nil {
t.Fatalf("Could not compute relative path(%v, %v); error: %v", rootDir, path, err)
}

if _, ok := excludes[relPath]; ok {
return filepath.SkipDir
}

// skip directories
if info.IsDir() {
return nil
}

// Skip non YAML files
ext := filepath.Ext(info.Name())

if ext != ".yaml" && ext != ".yml" {
return nil
}
data, err := ioutil.ReadFile(path)

if err != nil {
t.Errorf("Error reading %v; error: %v", path, err)
}

u := &unstructured.Unstructured{}

if err = yaml.Unmarshal(data, u); err != nil {
if match, _ := regexp.MatchString(".*Object 'Kind' is missing.*", err.Error()); match {
t.Logf("Skipping %v; not a K8s resource;", path)
return nil
}
t.Errorf("Error unmarshaling %v; error: %v", path, err)
}

if _, ok := u.UnstructuredContent()["status"]; ok {
t.Errorf("Path %v; has status field", path)
}
return nil
})

if err != nil {
t.Errorf("error walking the path %v; error: %v", rootDir, err)

}
}
6 changes: 0 additions & 6 deletions xgboost-job/xgboost-operator/base/crd.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -119,9 +119,3 @@ spec:
- replicaStatuses
type: object
version: v1alpha1
status:
acceptedNames:
kind: ""
plural: ""
conditions: []
storedVersions: []

0 comments on commit 9e38132

Please sign in to comment.