diff --git a/pkg/admit/confadmit.go b/pkg/admit/confadmit.go index 35eaf5b7..ed9132a6 100644 --- a/pkg/admit/confadmit.go +++ b/pkg/admit/confadmit.go @@ -43,11 +43,7 @@ func (validator *Validator) ValidateTenantConfig(responseWriter http.ResponseWri SendErroneousAdmissionResponse(responseWriter, admissionReview.Request, err) return } - err = mutateConfigManifest(newManifest) - if err != nil { - SendErroneousAdmissionResponse(responseWriter, admissionReview.Request, err) - return - } + mutateConfigManifest(newManifest) responseAdmissionReview := v1beta1.AdmissionReview { Response: CreateReviewResponseFromPatches(createPatchListFromConfigChanges(origNewManifest,newManifest)), } @@ -88,7 +84,7 @@ func validateConfig(oldManifest, newManifest *danmtypes.TenantConfig, opType v1b func createPatchListFromConfigChanges(origConfig danmtypes.TenantConfig, changedConfig *danmtypes.TenantConfig) []Patch { patchList := make([]Patch, 0) var hostDevicesPatch string - if !reflect.DeepEqual(origConfig.HostDevices, changedConfig.HostDevices) { + if !reflect.DeepEqual(origConfig.HostDevices, changedConfig.HostDevices) && len(origConfig.HostDevices) != 0 && len(changedConfig.HostDevices) != 0 { hostDevicesPatch = `[` for ifaceIndex, ifaceConf := range changedConfig.HostDevices { if ifaceIndex != 0 { @@ -105,17 +101,14 @@ func createPatchListFromConfigChanges(origConfig danmtypes.TenantConfig, changed return patchList } -func mutateConfigManifest(tconf *danmtypes.TenantConfig) error { +func mutateConfigManifest(tconf *danmtypes.TenantConfig) { for ifaceIndex, ifaceConf := range tconf.HostDevices { //We don't want to either re-init existing allocations, or unnecessarily create arrays for non-virtual networks if ifaceConf.Alloc != "" || ifaceConf.VniType == "" { continue } - bitArray, err := bitarray.NewBitArray(MaxAllowedVni) - if err != nil { - return err - } + bitArray, _ := bitarray.NewBitArray(MaxAllowedVni) tconf.HostDevices[ifaceIndex].Alloc = bitArray.Encode() } - return nil + return } \ No newline at end of file diff --git a/pkg/admit/validators.go b/pkg/admit/validators.go index 1d34f944..fe5204ea 100644 --- a/pkg/admit/validators.go +++ b/pkg/admit/validators.go @@ -107,7 +107,7 @@ func validateNetworkId(oldManifest, newManifest *danmtypes.DanmNet, opType admis return errors.New("Spec.NetworkID mandatory parameter is missing!") } if len(newManifest.Spec.NetworkID) > MaxNidLength && IsTypeDynamic(newManifest.Spec.NetworkType) { - return errors.New("Spec.NetworkID cannot be longer than 12 characters (otherwise VLAN and VxLAN host interface creation might fail)!") + return errors.New("Spec.NetworkID cannot be longer than " + strconv.Itoa(MaxNidLength) + " characters (otherwise VLAN and VxLAN host interface creation might fail)!") } return nil } @@ -151,7 +151,7 @@ func validateTenantconfig(oldManifest, newManifest *danmtypes.TenantConfig, opTy return errors.New("neither NetworkID, nor NetworkType can be empty in a NetworkID mapping!") } if len(nId) > MaxNidLength && IsTypeDynamic(nType) { - return errors.New("NetworkID:" + nId + " cannot be longer than 12 characters (otherwise VLAN and VxLAN host interface creation might fail)!") + return errors.New("NetworkID:" + nId + " cannot be longer than " + strconv.Itoa(MaxNidLength) + " characters (otherwise VLAN and VxLAN host interface creation might fail)!") } } return nil diff --git a/test/stubs/http/responsewriter_stub.go b/test/stubs/http/responsewriter_stub.go index 1d8ef1b2..3de02033 100644 --- a/test/stubs/http/responsewriter_stub.go +++ b/test/stubs/http/responsewriter_stub.go @@ -1,12 +1,9 @@ package http import ( -//"log" -//"fmt" -"bytes" + "bytes" "errors" "io/ioutil" -// "encoding/json" "net/http" "k8s.io/api/admission/v1beta1" "k8s.io/apimachinery/pkg/runtime" @@ -29,7 +26,6 @@ func (writer *ResponseWriterStub) Header() http.Header { } func (writer *ResponseWriterStub) Write(response []byte) (int, error) { -// log.Println("write was here, with the object:" + string(response)) writer.Response = response return 200, nil } @@ -42,9 +38,7 @@ func (writer *ResponseWriterStub) GetAdmissionResponse() (*v1beta1.AdmissionResp if writer.Response == nil { return nil, errors.New("no response was sent") } -// log.Println("get was here, the saved object is:" + string(writer.Response)) review := v1beta1.AdmissionReview{} -// err := json.Unmarshal(writer.Response, &response) reader := bytes.NewReader(writer.Response) readCloser := ioutil.NopCloser(reader) payload, err := ioutil.ReadAll(readCloser) @@ -54,6 +48,5 @@ func (writer *ResponseWriterStub) GetAdmissionResponse() (*v1beta1.AdmissionResp codecs := serializer.NewCodecFactory(runtime.NewScheme()) deserializer := codecs.UniversalDeserializer() _, _, err = deserializer.Decode(payload, nil, &review) -// fmt.Printf("after unmarshal the saved object is: %+v\n", review) return review.Response, err } \ No newline at end of file diff --git a/test/utils/utils.go b/test/utils/utils.go index 29d39bd3..9488e28f 100644 --- a/test/utils/utils.go +++ b/test/utils/utils.go @@ -75,4 +75,3 @@ func GetTconf(tconfName string, tconfSet []danmtypes.TenantConfig) *danmtypes.Te } return nil } - diff --git a/test/uts/admit_tests/confadmit_test.go b/test/uts/admit_tests/confadmit_test.go index 0aa9d584..b3442e9a 100644 --- a/test/uts/admit_tests/confadmit_test.go +++ b/test/uts/admit_tests/confadmit_test.go @@ -86,21 +86,64 @@ var ( }, }, danmtypes.TenantConfig { - ObjectMeta: meta_v1.ObjectMeta {Name: "manual-alloc"},TypeMeta: meta_v1.TypeMeta {Kind: "TenantConfig"}, + ObjectMeta: meta_v1.ObjectMeta {Name: "manual-alloc-old"},TypeMeta: meta_v1.TypeMeta {Kind: "TenantConfig"}, HostDevices: []danmtypes.IfaceProfile { danmtypes.IfaceProfile{Name: "ens4", VniType: "vxlan", VniRange: "700-710", Alloc: utils.AllocFor5k}, - danmtypes.IfaceProfile{Name: "ens5", VniType: "vxlan", VniRange: "700-710"}, }, }, danmtypes.TenantConfig { - ObjectMeta: meta_v1.ObjectMeta {Name: "tconf"}, + ObjectMeta: meta_v1.ObjectMeta {Name: "manual-alloc"},TypeMeta: meta_v1.TypeMeta {Kind: "TenantConfig"}, HostDevices: []danmtypes.IfaceProfile { danmtypes.IfaceProfile{Name: "ens4", VniType: "vxlan", VniRange: "700-710", Alloc: utils.AllocFor5k}, - danmtypes.IfaceProfile{Name: "ens4", VniType: "vlan", VniRange: "200,500-510", Alloc: utils.AllocFor5k}, - danmtypes.IfaceProfile{Name: "ens6", VniType: "vxlan", VniRange: "1200-1300", Alloc: utils.AllocFor5k}, - danmtypes.IfaceProfile{Name: "nokia.k8s.io/sriov_ens1f0", VniType: "vlan", VniRange: "1500-1550", Alloc: utils.AllocFor5k}, - danmtypes.IfaceProfile{Name: "nokia.k8s.io/sriov_ens1f0", VniType: "vxlan", VniRange: "1600-1650", Alloc: utils.AllocFor5k}, - }, + danmtypes.IfaceProfile{Name: "nokia.k8s.io/sriov_ens1f0", VniType: "vlan", VniRange: "700-710"}, + }, + }, + danmtypes.TenantConfig { + ObjectMeta: meta_v1.ObjectMeta {Name: "nonetype"},TypeMeta: meta_v1.TypeMeta {Kind: "TenantConfig"}, + NetworkIds: map[string]string { + "": "asd", + }, + }, + danmtypes.TenantConfig { + ObjectMeta: meta_v1.ObjectMeta {Name: "nonid"},TypeMeta: meta_v1.TypeMeta {Kind: "TenantConfig"}, + NetworkIds: map[string]string { + "flannel": "", + }, + }, + danmtypes.TenantConfig { + ObjectMeta: meta_v1.ObjectMeta {Name: "longnid"},TypeMeta: meta_v1.TypeMeta {Kind: "TenantConfig"}, + NetworkIds: map[string]string { + "flannel": "abcdefghijkl", + }, + }, + danmtypes.TenantConfig { + ObjectMeta: meta_v1.ObjectMeta {Name: "longnid-sriov"},TypeMeta: meta_v1.TypeMeta {Kind: "TenantConfig"}, + NetworkIds: map[string]string { + "flannel": "abcdefghijkl", + "sriov": "abcdefghijkl", + }, + }, + danmtypes.TenantConfig { + ObjectMeta: meta_v1.ObjectMeta {Name: "shortnid"},TypeMeta: meta_v1.TypeMeta {Kind: "TenantConfig"}, + NetworkIds: map[string]string { + "flannel": "abcdefghijkl", + "sriov": "abcdefghijk", + }, + }, + danmtypes.TenantConfig { + ObjectMeta: meta_v1.ObjectMeta {Name: "old-iface"},TypeMeta: meta_v1.TypeMeta {Kind: "TenantConfig"}, + HostDevices: []danmtypes.IfaceProfile { + danmtypes.IfaceProfile{Name: "ens4", VniType: "vxlan", VniRange: "900-4999,5000", Alloc: utils.AllocFor5k}, + }, + }, + danmtypes.TenantConfig { + ObjectMeta: meta_v1.ObjectMeta {Name: "new-iface"},TypeMeta: meta_v1.TypeMeta {Kind: "TenantConfig"}, + HostDevices: []danmtypes.IfaceProfile { + danmtypes.IfaceProfile{Name: "ens4", VniType: "vxlan", VniRange: "900-4999,5000", Alloc: utils.AllocFor5k}, + }, + NetworkIds: map[string]string { + "flannel": "flannel", + }, }, } ) @@ -111,21 +154,28 @@ var validateTconfTcs = []struct { newTconfName string opType v1beta1.Operation isErrorExpected bool + expectedPatches string }{ - {"emptyRequest", "", "", "", true}, - {"malformedOldObject", "malformed", "", "", true}, - {"malformedNewObject", "", "malformed", "", true}, - {"objectWithInvalidType", "", "invalid-type", "", true}, - {"emptyCofig", "", "empty-config", "", true}, - {"interfaceProfileWithoutName", "", "noname", "", true}, - {"interfaceProfileWithoutVniRange", "", "norange", "", true}, - {"interfaceProfileWithoutVniType", "", "notype", "", true}, - {"interfaceProfileWithInvalidVniType", "", "invalid-vni-type", "", true}, - {"interfaceProfileWithInvalidVniValue", "", "invalid-vni-value", "", true}, - {"interfaceProfileWithInvalidVniRange", "", "invalid-vni-range", "", true}, - {"interfaceProfileWithValidVniRange", "", "valid-vni-range", "", false}, - {"interfaceProfileWithSetAlloc", "", "manual-alloc", v1beta1.Create, true}, - {"interfaceProfileChangeWithAlloc", "", "manual-alloc", v1beta1.Update, false}, + {"emptyRequest", "", "", "", true, "empty"}, + {"malformedOldObject", "malformed", "", "", true, "empty"}, + {"malformedNewObject", "", "malformed", "", true, "empty"}, + {"objectWithInvalidType", "", "invalid-type", "", true, "empty"}, + {"emptyCofig", "", "empty-config", "", true, "empty"}, + {"interfaceProfileWithoutName", "", "noname", "", true, "empty"}, + {"interfaceProfileWithoutVniRange", "", "norange", "", true, "empty"}, + {"interfaceProfileWithoutVniType", "", "notype", "", true, "empty"}, + {"interfaceProfileWithInvalidVniType", "", "invalid-vni-type", "", true, "empty"}, + {"interfaceProfileWithInvalidVniValue", "", "invalid-vni-value", "", true, "empty"}, + {"interfaceProfileWithInvalidVniRange", "", "invalid-vni-range", "", true, "empty"}, + {"interfaceProfileWithValidVniRange", "", "valid-vni-range", "", false, "1"}, + {"interfaceProfileWithSetAlloc", "", "manual-alloc", v1beta1.Create, true, "empty"}, + {"interfaceProfileChangeWithAlloc", "manual-alloc-old", "manual-alloc", v1beta1.Update, false, "1"}, + {"networkIdWithoutKey", "", "nonid", "", true, "empty"}, + {"networkIdWithoutValue", "", "nonetype", "", true, "empty"}, + {"longNidWithStaticNeType", "", "longnid", "", false, "empty"}, + {"longNidWithDynamicNeType", "", "longnid-sriov", "", true, "empty"}, + {"okayNids", "", "shortnid", "", false, "empty"}, + {"noChangeInIfaces", "old-iface", "new-iface", v1beta1.Update, false, "empty"}, } func TestValidateTenantConfig(t *testing.T) { @@ -141,7 +191,7 @@ func TestValidateTenantConfig(t *testing.T) { return } validator.ValidateTenantConfig(writerStub, request) - err = validateHttpResponse(writerStub, tc.isErrorExpected) + err = validateHttpResponse(writerStub, tc.isErrorExpected, tc.expectedPatches) if err != nil { t.Errorf("Received HTTP Response did not match expectation, because:%v", err) return @@ -186,7 +236,7 @@ func canItMalform(config *danmtypes.TenantConfig) []byte { return bytes } -func validateHttpResponse(writer *httpstub.ResponseWriterStub, isErrorExpected bool) error { +func validateHttpResponse(writer *httpstub.ResponseWriterStub, isErrorExpected bool, expectedPatches string) error { if writer.RespHeader.Get("Content-Type") != "application/json" { return errors.New("Content-Type is not set to application/json in the HTTP Header") } @@ -198,7 +248,6 @@ func validateHttpResponse(writer *httpstub.ResponseWriterStub, isErrorExpected b if response.Allowed { return errors.New("request would have been admitted but we expected an error") } -// fmt.Printf("lofasz: %+v\n", response) if response.Result.Message == "" { return errors.New("a faulty response was sent without explanation") } @@ -210,5 +259,26 @@ func validateHttpResponse(writer *httpstub.ResponseWriterStub, isErrorExpected b return errors.New("an unnecessary Result message is put into a successful response") } } + if expectedPatches != "" { + return validatePatches(response, expectedPatches) + } return nil -} \ No newline at end of file +} + +func validatePatches(response *v1beta1.AdmissionResponse, expectedPatches string) error { + if expectedPatches == "empty" { + if response.Patch != nil { + return errors.New("did not expect any patches but some were included in the admission response") + } + return nil + } + var patches []admit.Patch + err := json.Unmarshal(response.Patch, &patches) + if err != nil { + return err + } + if len(patches) != 1 { + return errors.New("received number of patches was not the expected 1") + } + return nil +}