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

Handled IPAM policy properly to fully support strict IPAM #619

Merged

Conversation

Ex4amp1e
Copy link

Issue: #614
Problem: strict IMAP has not been working for dual stack NSE
Solution: Place strict IPAM policy over group policy if it is configured by ENV

How to test
Setup NSE env

        - name: NSM_IPAM_POLICY
          value: strict
        - name: NSM_CIDR_PREFIX
          value: 172.16.1.100/27,2001:db8::/116

Signed-off-by: Vladislav Byrgazov <vladislav.byrgazov@xored.com>
Vladislav Byrgazov added 2 commits October 11, 2024 13:33
Signed-off-by: Vladislav Byrgazov <vladislav.byrgazov@xored.com>
Signed-off-by: Vladislav Byrgazov <vladislav.byrgazov@xored.com>
main.go Outdated
@@ -94,7 +92,7 @@ type Config struct {
ConnectTo url.URL `default:"unix:///var/lib/networkservicemesh/nsm.io.sock" desc:"url to connect to" split_words:"true"`
MaxTokenLifetime time.Duration `default:"10m" desc:"maximum lifetime of tokens" split_words:"true"`
RegistryClientPolicies []string `default:"etc/nsm/opa/common/.*.rego,etc/nsm/opa/registry/.*.rego,etc/nsm/opa/client/.*.rego" desc:"paths to files and directories that contain registry client policies" split_words:"true"`
IPAMPolicy ipamPolicyFunc `default:"polite" desc:"defines NSE's IPAM Policy. Possible values: polite, strict. Polite policy accepts any addresses sent by client. Strict policy resets ip_context if any of the client's addresses doesn't match endpoint's CIDR." split_words:"true"`
IPAMPolicy string `default:"polite" desc:"defines NSE's IPAM Policy. Possible values: polite, strict. Polite policy accepts any addresses sent by client. Strict policy resets ip_context if any of the client's addresses doesn't match endpoint's CIDR." split_words:"true"`
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
IPAMPolicy string `default:"polite" desc:"defines NSE's IPAM Policy. Possible values: polite, strict. Polite policy accepts any addresses sent by client. Strict policy resets ip_context if any of the client's addresses doesn't match endpoint's CIDR." split_words:"true"`
IPAMPolicy newIPAMFunc `default:"polite" desc:"defines NSE's IPAM Policy. Possible values: polite, strict. Polite policy accepts any addresses sent by client. Strict policy resets ip_context if any of the client's addresses doesn't match endpoint's CIDR." split_words:"true"`

Copy link
Author

Choose a reason for hiding this comment

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

Done

@@ -111,23 +109,6 @@ type Config struct {
PprofListenOn string `default:"localhost:6060" desc:"pprof URL to ListenAndServe" split_words:"true"`
}

Copy link
Member

@denis-tingaikin denis-tingaikin Oct 16, 2024

Choose a reason for hiding this comment

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

Suggested change
type newIPAMFunc func(...*net.IPNet) networkservice.NetworkServiceServer
// Decode takes a string IPAM Policy and returns the IPAM Policy func
func (f *newIPAMFunc) Decode(policy string) error {
switch config.IPAMPolicy {
case "strict":
*f = func(cidrPrefix [][]*net.IPNet) {
var ipnetList []*net.IPNet
for _, group := range cidrPrefix {
ipnetList = append(ipnetList, group...)
}
return strictipam.NewServer(func(i ...*net.IPNet) networkservice.NetworkServiceServer {
return groupipam.NewServer(cidrPrefix)
}, ipnetList...)
case "polite":
*f = func(cidrPrefix [][]*net.IPNet) {
return groupipam.NewServer(cidrPrefix)
}
default:
logrus.Fatalf("not a valid IPAM Policy: %s", config.IPAMPolicy)
}
}

Copy link
Author

Choose a reason for hiding this comment

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

Done

main.go Outdated
Comment on lines 223 to 238
ipnetList := []*net.IPNet{}
for _, group := range config.CidrPrefix {
ipnetList = append(ipnetList, group...)
}
var IPAMServer networkservice.NetworkServiceServer

switch config.IPAMPolicy {
case "strict":
IPAMServer = strictipam.NewServer(func(i ...*net.IPNet) networkservice.NetworkServiceServer {
return groupipam.NewServer(config.CidrPrefix)
}, ipnetList...)
case "polite":
IPAMServer = groupipam.NewServer(config.CidrPrefix)
default:
logrus.Fatalf("not a valid IPAM Policy: %s", config.IPAMPolicy)
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
ipnetList := []*net.IPNet{}
for _, group := range config.CidrPrefix {
ipnetList = append(ipnetList, group...)
}
var IPAMServer networkservice.NetworkServiceServer
switch config.IPAMPolicy {
case "strict":
IPAMServer = strictipam.NewServer(func(i ...*net.IPNet) networkservice.NetworkServiceServer {
return groupipam.NewServer(config.CidrPrefix)
}, ipnetList...)
case "polite":
IPAMServer = groupipam.NewServer(config.CidrPrefix)
default:
logrus.Fatalf("not a valid IPAM Policy: %s", config.IPAMPolicy)
}

Copy link
Author

Choose a reason for hiding this comment

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

Done

main.go Outdated
responderEndpoint := endpoint.NewServer(ctx,
spiffejwt.TokenGeneratorFunc(source, config.MaxTokenLifetime),
endpoint.WithName(config.Name),
endpoint.WithAuthorizeServer(authorize.NewServer()),
endpoint.WithAdditionalFunctionality(
onidle.NewServer(ctx, cancel, config.IdleTimeout),
groupipam.NewServer(config.CidrPrefix, groupipam.WithCustomIPAMServer(config.IPAMPolicy)),
IPAMServer,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
IPAMServer,
config.newIPAMFunc(config.CidrPrefix),

Copy link
Author

Choose a reason for hiding this comment

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

Done

Signed-off-by: Vladislav Byrgazov <vladislav.byrgazov@xored.com>
@Ex4amp1e Ex4amp1e force-pushed the fix-strict-policy-conflicts branch from 8d222f1 to 300690b Compare October 17, 2024 05:32
Signed-off-by: Vladislav Byrgazov <vladislav.byrgazov@xored.com>
@denis-tingaikin denis-tingaikin merged commit aebfa96 into networkservicemesh:main Oct 17, 2024
13 checks passed
nsmbot pushed a commit to networkservicemesh/deployments-k8s that referenced this pull request Oct 17, 2024
…d-nse-icmp-responder@main

PR link: networkservicemesh/cmd-nse-icmp-responder#619

Commit: aebfa96
Author: Denis Tingaikin
Date: 2024-10-17 05:42:57 -0400
Message:
  - Merge pull request #619 from Ex4amp1e/fix-strict-policy-conflicts
Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
nsmbot added a commit to networkservicemesh/deployments-k8s that referenced this pull request Oct 17, 2024
…d-nse-icmp-responder@main (#12424)

PR link: networkservicemesh/cmd-nse-icmp-responder#619

Commit: aebfa96
Author: Denis Tingaikin
Date: 2024-10-17 05:42:57 -0400
Message:
  - Merge pull request #619 from Ex4amp1e/fix-strict-policy-conflicts

Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
Co-authored-by: NSMBot <nsmbot@networkservicmesh.io>
nsmbot pushed a commit that referenced this pull request Oct 17, 2024
…k-sriov@main

PR link: networkservicemesh/sdk-sriov#619

Commit: 3a8c515
Author: Network Service Mesh Bot
Date: 2024-10-17 12:20:56 -0500
Message:
  - Update go.mod and go.sum to latest version from networkservicemesh/sdk-kernel@main (#619)
PR link: networkservicemesh/sdk-kernel#691
Commit: 745014a
Author: Network Service Mesh Bot
Date: 2024-10-17 12:17:46 -0500
Message:
    - Update go.mod and go.sum to latest version from networkservicemesh/sdk@main (#691)
PR link: networkservicemesh/sdk#1535
Commit: 21c4d98
Author: dependabot[bot]
Date: 2024-10-17 13:15:20 -0400
Message:
        - Bump github.com/nats-io/nats-server/v2 from 2.8.2 to 2.9.23 (#1535)
Bumps [github.com/nats-io/nats-server/v2](https://github.com/nats-io/nats-server) from 2.8.2 to 2.9.23.
- [Release notes](https://github.com/nats-io/nats-server/releases)
- [Changelog](https://github.com/nats-io/nats-server/blob/main/.goreleaser.yml)
- [Commits](nats-io/nats-server@v2.8.2...v2.9.23)
---
updated-dependencies:
- dependency-name: github.com/nats-io/nats-server/v2
  dependency-type: indirect
...
Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
nsmbot added a commit that referenced this pull request Oct 17, 2024
…k-sriov@main (#625)

PR link: networkservicemesh/sdk-sriov#619

Commit: 3a8c515
Author: Network Service Mesh Bot
Date: 2024-10-17 12:20:56 -0500
Message:
  - Update go.mod and go.sum to latest version from networkservicemesh/sdk-kernel@main (#619)
PR link: networkservicemesh/sdk-kernel#691
Commit: 745014a
Author: Network Service Mesh Bot
Date: 2024-10-17 12:17:46 -0500
Message:
    - Update go.mod and go.sum to latest version from networkservicemesh/sdk@main (#691)
PR link: networkservicemesh/sdk#1535
Commit: 21c4d98
Author: dependabot[bot]
Date: 2024-10-17 13:15:20 -0400
Message:
        - Bump github.com/nats-io/nats-server/v2 from 2.8.2 to 2.9.23 (#1535)
Bumps [github.com/nats-io/nats-server/v2](https://github.com/nats-io/nats-server) from 2.8.2 to 2.9.23.
- [Release notes](https://github.com/nats-io/nats-server/releases)
- [Changelog](https://github.com/nats-io/nats-server/blob/main/.goreleaser.yml)
- [Commits](nats-io/nats-server@v2.8.2...v2.9.23)
---
updated-dependencies:
- dependency-name: github.com/nats-io/nats-server/v2
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
Co-authored-by: NSMBot <nsmbot@networkservicmesh.io>
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.

2 participants