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.

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

Unable to create vlan or vlantemplate with vlan ID 0 #5

Open
rvichery opened this issue Mar 4, 2018 · 4 comments
Open

Unable to create vlan or vlantemplate with vlan ID 0 #5

rvichery opened this issue Mar 4, 2018 · 4 comments
Assignees
Labels

Comments

@rvichery
Copy link

rvichery commented Mar 4, 2018

When creating a vlan or vlantemplate with VLAN ID 0, the program throws the error below:

{"title": "Invalid input. Value cannot be null", "description": "This value cannot be null"}

Both vlan and vlantemplate are configured to omit the VLAN ID if empty. Golang interprets int(0) as empty, and thus the value is not passed to the API.

type VLAN struct {
	ID                                    string `json:"ID,omitempty"`
	ParentID                              string `json:"parentID,omitempty"`
	ParentType                            string `json:"parentType,omitempty"`
	Owner                                 string `json:"owner,omitempty"`
	Value                                 int    `json:"value,omitempty"`
	LastUpdatedBy                         string `json:"lastUpdatedBy,omitempty"`
	GatewayID                             string `json:"gatewayID,omitempty"`
	Readonly                              bool   `json:"readonly"`
	TemplateID                            string `json:"templateID,omitempty"`
	PermittedAction                       string `json:"permittedAction,omitempty"`
	Description                           string `json:"description,omitempty"`
	Restricted                            bool   `json:"restricted"`
	EntityScope                           string `json:"entityScope,omitempty"`
	VportID                               string `json:"vportID,omitempty"`
	IsUplink                              bool   `json:"isUplink"`
	UseUserMnemonic                       bool   `json:"useUserMnemonic"`
	UserMnemonic                          string `json:"userMnemonic,omitempty"`
	AssociatedBGPProfileID                string `json:"associatedBGPProfileID,omitempty"`
	AssociatedConnectionType              string `json:"associatedConnectionType,omitempty"`
	AssociatedEgressQOSPolicyID           string `json:"associatedEgressQOSPolicyID,omitempty"`
	AssociatedIngressOverlayQoSPolicerID  string `json:"associatedIngressOverlayQoSPolicerID,omitempty"`
	AssociatedIngressQOSPolicyID          string `json:"associatedIngressQOSPolicyID,omitempty"`
	AssociatedIngressUnderlayQoSPolicerID string `json:"associatedIngressUnderlayQoSPolicerID,omitempty"`
	AssociatedUplinkConnectionID          string `json:"associatedUplinkConnectionID,omitempty"`
	AssociatedVSCProfileID                string `json:"associatedVSCProfileID,omitempty"`
	Status                                string `json:"status,omitempty"`
	DucVlan                               bool   `json:"ducVlan"`
	ExternalID                            string `json:"externalID,omitempty"`
	Type                                  string `json:"type,omitempty"`
}

I am going to open an issue and work on a pull request on monolithe to fix this across all rest objects.

@pdellaert
Copy link
Member

Thanks for reporting this, and looking into it!

@rvichery
Copy link
Author

I looked into this issue, and here are my findings:

Removing the omitempty tag on all bool and int attributes seemed to do the trick initially. But I discovered that it can also introduce issues on int.

For example, in enterprise.go the local_as attribute is an int and is not a required attribute. Thus, if a struct is created without a local_as value, it will automatically initialize the attribute with the value 0, which is not allowed for this specific attribute. The API will reject the request because the attribute does not resepct the constraint min:1, max:65535

Here is an example of what I just described:

package main

import "fmt"
import "encoding/json"

type Test struct {
	ID                                     string        `json:"ID,omitempty"`
	LocalAS				       int           `json:"localAS"`
}

func main() {
	d, err := json.Marshal(Test{ID: "1234"})
	fmt.Println(string(d), err)
	d2, err := json.Marshal(Test{ID: "1234", LocalAS: 1})
	fmt.Println(string(d2), err)
}

Output

{"ID":"1234","localAS":0} <nil>
{"ID":"1234","localAS":1} <nil>

We could put the omitempty tag only on optional attributes but there will still be an issue if the attribute is optional and accept 0 as a value.

I will explore two options:

  • The recommendation from the Golang community is to use pointers in those type of structures, because omitempty will only check if the pointer is null or not, allowing for any int or bool value.I know that moving to a pointer based approach will impact any project currently using this library.
  • Implement a custom MarshalJSON and UnmarshalJSON function to encode/decode all our structures.

The first option is easier to implement but require to change many project implementation.
The second will require more work but it will avoid updating projects developed using this library.

What are your thoughts about these options ?

@hellt
Copy link

hellt commented Jun 18, 2019

The first option is easier to implement but require to change many project implementation.

I would argue that any project is using vspk go successfully given that they aren't able to create an untagged VLAN.

@pdellaert any progress with this as we are on a verge of 6.0?

@pdellaert
Copy link
Member

@hellt , sorry... No, since Remy left, the subject has been very silent and i'm lacking the expertise to solve this issue myself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants