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

policy: limit the number of L4 ports to 40 #1406

Merged
merged 3 commits into from Sep 1, 2017
Merged

policy: limit the number of L4 ports to 40 #1406

merged 3 commits into from Sep 1, 2017

Conversation

aalemayhu
Copy link
Contributor

Closes: #922 (L4 policy with more than 40 ports doesn't work)

@aalemayhu aalemayhu added wip and removed wip labels Aug 28, 2017
@aalemayhu aalemayhu requested review from aanm and ashwinp August 31, 2017 09:59
@@ -324,6 +324,9 @@ The ``toPorts`` field takes a ``PortProtocol`` structure which is defined as fol
Protocol string `json:"protocol,omitempty"`
}

.. note:: There is currently a max limit of 40 ports. This might change in the
future when support for ranges are added.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is added

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

@@ -27,7 +26,7 @@ echo "------ checking cilium status ------"
cilium status

echo "------ creating Docker network of type Cilium ------"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the echo statements prior to the calls to this helper function as the echo statement is already part of the helper function.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was actually done earlier but probably pushed from the wrong machine. Should be gone now.

Also updated policy import test with check for ingress but the
validation is performed on the `PortRule` so works for egress as well.

Closes: #922 (L4 policy with more than 40 ports doesn't work)
Signed-off-by: Alexander Alemayhu <alexander@alemayhu.com>
Valid json is not enough to detect all syntax errors, f. ex. when
using {to,from}CIDR.

Signed-off-by: Alexander Alemayhu <alexander@alemayhu.com>
This fixes some local failures I have seen when running
`16-cidr-ingress-policy`.
@aalemayhu aalemayhu merged commit 0e0b62d into cilium:master Sep 1, 2017
@aalemayhu aalemayhu deleted the 40-ports-policy-limit branch September 1, 2017 22:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants