feat(control-loop/ensure-networks) Add notion of control loop, ensure network endpoints every 30 minutes by leo-scalingo · Pull Request #329 · Scalingo/sand

leo-scalingo · 2026-07-17T13:34:36Z

Also add ability to trigger the logics by CLI/API

Fixes #328

… every 30 minutes

…rtng the service

semgrep-code-scalingo · 2026-07-17T13:38:01Z

+		if err != nil {
+			// If the netns path no longer exists, deactivate the endpoint. Other
+			// activation errors should not prevent the remaining endpoints from being ensured.
+			if os.IsNotExist(errors.Cause(err)) {


Semgrep identified an issue in your code:

A wrapped file-not-found error from erepo.Activate(...) may not be recognized here, so missing endpoint.TargetNetnsPath can skip erepo.Deactivate(...). An attacker who can cause stale netns paths during restore could leave orphaned endpoint network state behind.

More details about this

erepo.Activate(...) returns an error, and this code decides whether to clean up the endpoint by checking os.IsNotExist(errors.Cause(err)). That check only looks at the unwrapped cause, so a wrapped not exist error can be missed and the code will take the failed to ensure endpoint path instead of calling erepo.Deactivate(...).

A plausible abuse case is: 1) an attacker causes endpoint.TargetNetnsPath to point to a namespace path that has already been removed, 2) erepo.Activate(endpointCtx, network, endpoint, ...) returns a wrapped file-not-found error for that path, 3) this os.IsNotExist(errors.Cause(err)) check fails to recognize it, and 4) the stale endpoint is left active because erepo.Deactivate(endpointCtx, network, endpoint) is never called. In a system restoring many endpoints, repeatedly triggering this on selected endpoint.NetworkID / endpoint.TargetNetnsPath values can leave orphaned network state behind, which can keep unexpected connectivity or policy state attached to containers that should have been cleaned up.

To resolve this comment:

✨ Commit fix suggestion

Suggested change

if os.IsNotExist(errors.Cause(err)) {

if errors.Is(errors.Cause(err), fs.ErrNotExist) {

View step-by-step instructions

Replace the os.IsNotExist(...) check with the standard library errors.Is(...) check against the not-exist sentinel error.
Change os.IsNotExist(errors.Cause(err)) to stderrors.Is(err, fs.ErrNotExist).

Stop unwrapping the error with errors.Cause(...) in this condition.
errors.Is already walks wrapped errors, so it can match the original not-exist error through errors.Wrap, fmt.Errorf(... %w ...), and similar wrappers.

Add the standard library imports needed for the new check.
If this file already imports github.com/pkg/errors as errors, import the standard library package with an alias such as stderrors "errors", and add io/fs for fs.ErrNotExist.

Keep the existing deactivate path the same, and only update the condition.
The resulting condition should look like if stderrors.Is(err, fs.ErrNotExist) { ... }.

Alternatively, if this file does not import github.com/pkg/errors with the name errors, use the standard library package directly as errors.Is(err, fs.ErrNotExist).

💬 Ignore this finding

Reply with Semgrep commands to ignore this finding.

/fp <comment> for false positive

/ar <comment> for acceptable risk

/other <comment> for all other reasons

Alternatively, triage in Semgrep AppSec Platform to ignore the finding created by os-error-is-not-exist.

_{You can view more details about this finding in the Semgrep AppSec Platform.}

I second this

EtienneM

Would it be possible to

EtienneM · 2026-07-17T13:49:28Z

-	return nil
+func setupEndpointEnsureCron(ctx context.Context, c *config.Config, repo network.Repository, erepo endpoint.Repository) (func(), error) {
+	return cronsetup.Setup(ctx, cronsetup.SetupOpts{
+		WithoutTelemetry: true,


question: is there a reason why you remove the telemtry?

Because it wasn't installed yet in sand, this will change with my next PR

EtienneM · 2026-07-17T13:49:41Z

+		WithoutTelemetry: true,
+		Jobs: []cronsetup.Job{
+			{
+				Name:   "ensure network endpoints",


question: is it safe to have a name with spaces in it?

You're right, poor review from my side

EtienneM · 2026-07-17T13:49:55Z

+				Name:   "ensure network endpoints",
+				Rhythm: "@every " + c.EndpointEnsureInterval.String(),
+				Func: func(ctx context.Context) error {
+					return node.EnsureNetworkEndpoints(ctx, c, repo, erepo)


issue: wrap the error

Ok I could wrap it, but it doesn't bring a lot of value at this level I'd say, but ok

EtienneM · 2026-07-17T13:55:35Z

+	if err != nil {
+		return err
+	}
+
+	err = client.NodeEnsureNetworkEndpoints(context.Background())
+	if err != nil {
+		return err
+	}


issue: wrap the errors

EtienneM · 2026-07-17T14:02:11Z

+
+func EnsureNetworkEndpoints(ctx context.Context, c *config.Config, repo network.Repository, erepo endpoint.Repository) error {
+	log := logger.Get(ctx)
+	ctx = logger.ToCtx(ctx, log)


Suggested change

ctx = logger.ToCtx(ctx, log)

EtienneM · 2026-07-17T14:02:53Z

+		endpointLog := log.WithFields(logrus.Fields{
+			"endpoint_id": endpoint.ID,
+		})
+		endpointCtx := logger.ToCtx(ctx, endpointLog)


Suggested change

endpointLog := log.WithFields(logrus.Fields{

"endpoint_id": endpoint.ID,

})

endpointCtx := logger.ToCtx(ctx, endpointLog)

ctx, log := logger.WithFieldsToCtx(ctx, logrus.Fields{

"endpoint_id": endpoint.ID,

})

EtienneM · 2026-07-17T14:03:19Z

+		endpointLog = endpointLog.WithFields(logrus.Fields{
+			"network_id":          endpoint.NetworkID,
+			"endpoint_netns_path": endpoint.TargetNetnsPath,
+		})
+		endpointCtx = logger.ToCtx(ctx, endpointLog)


Suggested change

endpointLog = endpointLog.WithFields(logrus.Fields{

"network_id": endpoint.NetworkID,

"endpoint_netns_path": endpoint.TargetNetnsPath,

})

endpointCtx = logger.ToCtx(ctx, endpointLog)

ctx, log = logger.WithFieldsToCtx(ctx, logrus.Fields{

"network_id": endpoint.NetworkID,

"endpoint_netns_path": endpoint.TargetNetnsPath,

})

EtienneM · 2026-07-17T14:04:18Z

+	log.Info("Ensure networks on node")
+
+	endpoints, err := erepo.List(ctx, map[string]string{"hostname": c.GetPeerHostname()})
+	if errors.Cause(err) == store.ErrNotFound {


suggestion: should we already switch to go-utils/errors/v3.Is, even though this service does not yet use our own error library?

briceamen · 2026-07-20T08:00:06Z

+	"github.com/Scalingo/sand/store"
+)
+
+func EnsureNetworkEndpoints(ctx context.Context, c *config.Config, repo network.Repository, erepo endpoint.Repository) error {


From what I understand EnsureNetworkEndpoints() is run by the cron every 30 minutes but also by the endpoint POST /node/ensure-network-endpoints what will happen if they collide and run at the same time ?

briceamen · 2026-07-20T08:01:10Z

+		if err != nil {
+			// If the netns path no longer exists, deactivate the endpoint. Other
+			// activation errors should not prevent the remaining endpoints from being ensured.
+			if os.IsNotExist(errors.Cause(err)) {


I second this

leo-scalingo requested a review from a team as a code owner July 17, 2026 13:34

leo-scalingo requested review from EtienneM and removed request for a team July 17, 2026 13:34

leo-scalingo self-assigned this Jul 17, 2026

leo-scalingo requested a review from briceamen July 17, 2026 13:34

leo-scalingo changed the title ~~feat(control-loop/ensure-networks) Add notion of control loop, ensure every 30 minutes~~ feat(control-loop/ensure-networks) Add notion of control loop, ensure network endpoints every 30 minutes Jul 17, 2026

leo-scalingo added 3 commits July 17, 2026 15:36

feat(control-loop/ensure-networks) Add notion of control loop, ensure…

db0c8b1

… every 30 minutes

Add API endpoint and CLI command to trigger it manually without resta…

4ed6d9b

…rtng the service

Update Changelog

346f5ee

leo-scalingo force-pushed the feat/328/control-loop-ensure branch from b64bc4d to 346f5ee Compare July 17, 2026 13:38

semgrep-code-scalingo Bot reviewed Jul 17, 2026

View reviewed changes

EtienneM requested changes Jul 17, 2026

View reviewed changes

briceamen reviewed Jul 20, 2026

View reviewed changes

	if os.IsNotExist(errors.Cause(err)) {
	if errors.Is(errors.Cause(err), fs.ErrNotExist) {

Uh oh!

Conversation

leo-scalingo commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

semgrep-code-scalingo Bot Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

EtienneM left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

leo-scalingo commented Jul 17, 2026 •

edited

Loading