diff --git a/api/protobuf-spec/github_com_openshift_origin_pkg_sdn_api_v1.proto b/api/protobuf-spec/github_com_openshift_origin_pkg_sdn_api_v1.proto index f957db3dcb31..d9716705803f 100644 --- a/api/protobuf-spec/github_com_openshift_origin_pkg_sdn_api_v1.proto +++ b/api/protobuf-spec/github_com_openshift_origin_pkg_sdn_api_v1.proto @@ -91,8 +91,7 @@ message HostSubnet { // Standard object's metadata. optional k8s.io.kubernetes.pkg.api.v1.ObjectMeta metadata = 1; - // Host is the name of the node. (This is redundant with the object's name, and this - // field is not actually used any more.) + // Host is the name of the node. (This is the same as the object's name, but both fields must be set.) optional string host = 2; // HostIP is the IP address to be used as a VTEP by other nodes in the overlay network diff --git a/api/swagger-spec/oapi-v1.json b/api/swagger-spec/oapi-v1.json index e835068c689c..1d5b7d627022 100644 --- a/api/swagger-spec/oapi-v1.json +++ b/api/swagger-spec/oapi-v1.json @@ -26657,7 +26657,7 @@ }, "host": { "type": "string", - "description": "Host is the name of the node. (This is redundant with the object's name, and this field is not actually used any more.)" + "description": "Host is the name of the node. (This is the same as the object's name, but both fields must be set.)" }, "hostIP": { "type": "string", diff --git a/api/swagger-spec/openshift-openapi-spec.json b/api/swagger-spec/openshift-openapi-spec.json index 91e7df4aabda..8d561cc8c059 100644 --- a/api/swagger-spec/openshift-openapi-spec.json +++ b/api/swagger-spec/openshift-openapi-spec.json @@ -50864,7 +50864,7 @@ "type": "string" }, "host": { - "description": "Host is the name of the node. (This is redundant with the object's name, and this field is not actually used any more.)", + "description": "Host is the name of the node. (This is the same as the object's name, but both fields must be set.)", "type": "string" }, "hostIP": { diff --git a/pkg/openapi/zz_generated.openapi.go b/pkg/openapi/zz_generated.openapi.go index 13983d04ec05..3df2994be45f 100644 --- a/pkg/openapi/zz_generated.openapi.go +++ b/pkg/openapi/zz_generated.openapi.go @@ -13228,7 +13228,7 @@ var OpenAPIDefinitions *common.OpenAPIDefinitions = &common.OpenAPIDefinitions{ }, "host": { SchemaProps: spec.SchemaProps{ - Description: "Host is the name of the node. (This is redundant with the object's name, and this field is not actually used any more.)", + Description: "Host is the name of the node. (This is the same as the object's name, but both fields must be set.)", Type: []string{"string"}, Format: "", }, diff --git a/pkg/sdn/api/v1/generated.proto b/pkg/sdn/api/v1/generated.proto index f957db3dcb31..d9716705803f 100644 --- a/pkg/sdn/api/v1/generated.proto +++ b/pkg/sdn/api/v1/generated.proto @@ -91,8 +91,7 @@ message HostSubnet { // Standard object's metadata. optional k8s.io.kubernetes.pkg.api.v1.ObjectMeta metadata = 1; - // Host is the name of the node. (This is redundant with the object's name, and this - // field is not actually used any more.) + // Host is the name of the node. (This is the same as the object's name, but both fields must be set.) optional string host = 2; // HostIP is the IP address to be used as a VTEP by other nodes in the overlay network diff --git a/pkg/sdn/api/v1/swagger_doc.go b/pkg/sdn/api/v1/swagger_doc.go index 1b8fb8d1d23e..5d5bcdb68b6a 100644 --- a/pkg/sdn/api/v1/swagger_doc.go +++ b/pkg/sdn/api/v1/swagger_doc.go @@ -79,7 +79,7 @@ func (EgressNetworkPolicySpec) SwaggerDoc() map[string]string { var map_HostSubnet = map[string]string{ "": "HostSubnet describes the container subnet network on a node. The HostSubnet object must have the same name as the Node object it corresponds to.", "metadata": "Standard object's metadata.", - "host": "Host is the name of the node. (This is redundant with the object's name, and this field is not actually used any more.)", + "host": "Host is the name of the node. (This is the same as the object's name, but both fields must be set.)", "hostIP": "HostIP is the IP address to be used as a VTEP by other nodes in the overlay network", "subnet": "Subnet is the CIDR range of the overlay network assigned to the node for its pods", } diff --git a/pkg/sdn/api/v1/types.go b/pkg/sdn/api/v1/types.go index f11ee43e847c..32411ee133f7 100644 --- a/pkg/sdn/api/v1/types.go +++ b/pkg/sdn/api/v1/types.go @@ -45,8 +45,7 @@ type HostSubnet struct { // Standard object's metadata. kapi.ObjectMeta `json:"metadata,omitempty" protobuf:"bytes,1,opt,name=metadata"` - // Host is the name of the node. (This is redundant with the object's name, and this - // field is not actually used any more.) + // Host is the name of the node. (This is the same as the object's name, but both fields must be set.) Host string `json:"host" protobuf:"bytes,2,opt,name=host"` // HostIP is the IP address to be used as a VTEP by other nodes in the overlay network HostIP string `json:"hostIP" protobuf:"bytes,3,opt,name=hostIP"` diff --git a/pkg/sdn/api/validation/validation.go b/pkg/sdn/api/validation/validation.go index d025e112d1a4..a2cbf0f8dc32 100644 --- a/pkg/sdn/api/validation/validation.go +++ b/pkg/sdn/api/validation/validation.go @@ -1,6 +1,7 @@ package validation import ( + "fmt" "net" "k8s.io/kubernetes/pkg/api/validation" @@ -85,6 +86,10 @@ func ValidateClusterNetworkUpdate(obj *sdnapi.ClusterNetwork, old *sdnapi.Cluste func ValidateHostSubnet(hs *sdnapi.HostSubnet) field.ErrorList { allErrs := validation.ValidateObjectMeta(&hs.ObjectMeta, false, path.ValidatePathSegmentName, field.NewPath("metadata")) + if hs.Host != hs.Name { + allErrs = append(allErrs, field.Invalid(field.NewPath("host"), hs.Host, fmt.Sprintf("must be the same as metadata.name: %q", hs.Name))) + } + if hs.Subnet == "" { // check if annotation exists, then let the Subnet field be empty if _, ok := hs.Annotations[sdnapi.AssignHostSubnetAnnotation]; !ok { @@ -117,8 +122,12 @@ func ValidateHostSubnetUpdate(obj *sdnapi.HostSubnet, old *sdnapi.HostSubnet) fi func ValidateNetNamespace(netnamespace *sdnapi.NetNamespace) field.ErrorList { allErrs := validation.ValidateObjectMeta(&netnamespace.ObjectMeta, false, path.ValidatePathSegmentName, field.NewPath("metadata")) + if netnamespace.NetName != netnamespace.Name { + allErrs = append(allErrs, field.Invalid(field.NewPath("netname"), netnamespace.NetName, fmt.Sprintf("must be the same as metadata.name: %q", netnamespace.Name))) + } + if err := sdnapi.ValidVNID(netnamespace.NetID); err != nil { - allErrs = append(allErrs, field.Invalid(field.NewPath("netID"), netnamespace.NetID, err.Error())) + allErrs = append(allErrs, field.Invalid(field.NewPath("netid"), netnamespace.NetID, err.Error())) } return allErrs } diff --git a/test/integration/etcd_storage_path_test.go b/test/integration/etcd_storage_path_test.go index 3cdda1ea65f7..caf15ba59bb7 100644 --- a/test/integration/etcd_storage_path_test.go +++ b/test/integration/etcd_storage_path_test.go @@ -11,7 +11,6 @@ import ( "time" kapi "k8s.io/kubernetes/pkg/api" - kubeerr "k8s.io/kubernetes/pkg/api/errors" "k8s.io/kubernetes/pkg/api/meta" "k8s.io/kubernetes/pkg/api/unversioned" "k8s.io/kubernetes/pkg/client/restclient" @@ -154,12 +153,12 @@ var etcdStorageData = map[unversioned.GroupVersionResource]struct { // -- // github.com/openshift/origin/pkg/sdn/api/v1 - gvr("", "v1", "netnamespaces"): { // This will fail to delete because meta.name != NetName but it is keyed off NetName - stub: `{"metadata": {"name": "nn1"}, "netid": 100, "netname": "networkname"}`, + gvr("", "v1", "netnamespaces"): { + stub: `{"metadata": {"name": "networkname"}, "netid": 100, "netname": "networkname"}`, expectedEtcdPath: "openshift.io/registry/sdnnetnamespaces/networkname", }, - gvr("", "v1", "hostsubnets"): { // This will fail to delete because meta.name != Host but it is keyed off Host - stub: `{"host": "hostname", "hostIP": "192.168.1.1", "metadata": {"name": "hs1"}, "subnet": "192.168.1.1/24"}`, + gvr("", "v1", "hostsubnets"): { + stub: `{"host": "hostname", "hostIP": "192.168.1.1", "metadata": {"name": "hostname"}, "subnet": "192.168.1.1/24"}`, expectedEtcdPath: "openshift.io/registry/sdnsubnets/hostname", }, gvr("", "v1", "clusternetworks"): { @@ -836,12 +835,7 @@ func (c *allClient) cleanup(all *[]cleanupData) error { mapping := (*all)[i].mapping if err := c.destroy(obj, mapping); err != nil { - if kubeerr.IsNotFound(err) && isInInvalidNameWhiteList(mapping) { - continue - } return err - } else if err == nil && isInInvalidNameWhiteList(mapping) { - return fmt.Errorf("Object %#v with mapping %#v should fail to delete if it is in the invalid name whitelist", obj, mapping) } } return nil @@ -939,15 +933,6 @@ func createSerializers(config restclient.ContentConfig) (*restclient.Serializers return s, nil } -// do NOT add anything to this - doing so means you wrote something that is broken -func isInInvalidNameWhiteList(mapping *meta.RESTMapping) bool { - switch mapping.GroupVersionKind.GroupVersion().WithResource(mapping.Resource) { - case gvr("", "v1", "netnamespaces"), gvr("", "v1", "hostsubnets"): // TODO figure out how to not whitelist these - return true - } - return false -} - func getFromEtcd(keys etcd.KeysAPI, path string) (*metaObject, error) { response, err := keys.Get(context.Background(), path, nil) if err != nil {