Disallow invalid VLAN/VNI value during network creation #1069
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Currently, netctl parse the pkgTag input as an integer directly. Therefore when user put a garbage in pkgTag, the conversion will default it to 0 instead. This patchset is to cast the input to string and perform further validation logic before we create network Signed-off-by: Kahou Lei <kalei@cisco.com>
chrisplo
reviewed
Nov 15, 2017
netctl/netctl.go
Outdated
| var pktTag int | ||
| var err error | ||
| if len(pktTagInString) > 0 { | ||
| pktTag, err = strconv.Atoi(pktTagInString) |
There was a problem hiding this comment.
nit, drop the declaration of err on 538 and use
pktTag, err := strconv.Atoi(pktTagInString)
There was a problem hiding this comment.
I can't as pktTag need to be used in line 560.
There was a problem hiding this comment.
just the err, pktTag still decalred on 537, not a blocker
|
This is the issue refer to contiv/install#182 |
dseevr
approved these changes
Nov 15, 2017
chrisplo
previously approved these changes
Nov 15, 2017
|
build PR |
|
isn't it more appropriate to change to cli.Intflag ?https://github.com/contiv/netplugin/blob/master/netctl/commands.go#L219 |
|
@rchirakk i thought pkt-tag can represent multiple VLAN IDs and VNIs and therefore it was using String for the command input? |
chrisplo
dismissed
their stale review
there seems to be a mismatch with expected input values and implementation
|
@chrisplo please review again |
|
build PR |
1 similar comment
|
build PR |
chrisplo
approved these changes
Nov 16, 2017
rchirakk
approved these changes
Nov 17, 2017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently, netctl parse the pkgTag input as an integer directly.
Therefore when user put a garbage in pkgTag, the conversion will
default it to 0 instead.
This patchset is to cast the input to string and perform further
validation logic before we create network
Signed-off-by: Kahou Lei kalei@cisco.com