Skip to content
This repository was archived by the owner on Jun 20, 2024. It is now read-only.

Fix weave-npc crash when named ports specified in the named ports #3375

Merged
merged 3 commits into from
Aug 17, 2018

Conversation

murali-reddy
Copy link
Contributor

Support for names ports in enhancement, but with this PR just adding more graceful failure.

Fixes #3032

Support for names ports in enhancement, but with this PR just adding more graceful failure.

Fixes #3032
Copy link
Contributor

@bboreham bboreham left a comment

Choose a reason for hiding this comment

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

Great! Couple of nits.

npc/analyser.go Outdated
if !allPorts {
for _, npProtocolPort := range ingressRule.Ports {
if _, err := strconv.Atoi(port(npProtocolPort.Port)); err != nil {
return nil, nil, nil, nil, errors.New("named ports in network policies in not support yet. Rejecting network policy from further processing")

This comment was marked as abuse.

This comment was marked as abuse.

npc/analyser.go Outdated
if !allPorts {
for _, npProtocolPort := range egressRule.Ports {
if _, err := strconv.Atoi(port(npProtocolPort.Port)); err != nil {
return nil, nil, nil, nil, errors.New("named ports in network policies in not support yet. Rejecting network policy from further processing")

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

@murali-reddy murali-reddy requested a review from brb August 14, 2018 10:22
npc/analyser.go Outdated
@@ -50,6 +51,14 @@ func (ns *ns) analysePolicy(policy *networkingv1.NetworkPolicy) (
// If From is empty or missing, this rule matches all sources
allSources := ingressRule.From == nil || len(ingressRule.From) == 0

if !allPorts {
for _, npProtocolPort := range ingressRule.Ports {

This comment was marked as abuse.

This comment was marked as abuse.

@brb brb added this to the 2.5 milestone Aug 17, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants