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

ROX-25218: Instruct Collector to not aggregate some subnets #1639

Merged
merged 8 commits into from
Jul 8, 2024

Conversation

ovalenti
Copy link
Contributor

@ovalenti ovalenti commented Apr 22, 2024

Description

This adds an environment variable providing Collector with a list of subnets to never aggregate (ROX_NON_AGGREGATED_NETWORKS).

As a side effect, this can be used to expand the list of standard private network prefixes with new ones.

Checklist

  • Investigated and inspected CI test results
  • Updated documentation accordingly

Testing

  • deployed a K8S cluster with 34.228.224.0/24 as the service CIDR
  • install ACS 4.4
  • the network graph shows no connection to central-db
  • update the collector image to the one produced by this PR. Set ROX_NON_AGGREGATED_NETWORKS=34.228.224.0/24
  • the network graph then shows central connecting to central-db

@ovalenti ovalenti self-assigned this Apr 22, 2024
@ovalenti ovalenti closed this May 14, 2024
@ovalenti ovalenti reopened this Jul 2, 2024
@ovalenti ovalenti marked this pull request as ready for review July 2, 2024 15:29
@ovalenti ovalenti requested a review from a team as a code owner July 2, 2024 15:29
Copy link
Collaborator

@Molter73 Molter73 left a comment

Choose a reason for hiding this comment

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

As discussed offline, maybe non aggregated networks/address might be a better name to detailed networks/address. We can use a synonym or shorten the words when needed.

Other than that the PR looks good, think it's a good step forward for how we handle networks, just left a couple minor comments.

collector/lib/CollectorConfig.cpp Outdated Show resolved Hide resolved
collector/lib/CollectorConfig.cpp Outdated Show resolved Hide resolved
collector/lib/ConnTracker.cpp Outdated Show resolved Hide resolved
@ovalenti ovalenti force-pushed the ovalenti/config_detailed_subnets branch from 4f9da10 to dd6048e Compare July 4, 2024 09:48
Copy link
Collaborator

@Molter73 Molter73 left a comment

Choose a reason for hiding this comment

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

LGTM!

@ovalenti ovalenti merged commit 0eb641f into master Jul 8, 2024
37 checks passed
@ovalenti ovalenti deleted the ovalenti/config_detailed_subnets branch July 8, 2024 09:53
@ovalenti ovalenti changed the title Instruct Collector to not aggregate some subnets ROX-25218: Instruct Collector to not aggregate some subnets Jul 10, 2024
ovalenti added a commit that referenced this pull request Jul 10, 2024
ROX_NON_AGGREGATED_NETWORKS can list subnets that should not
be aggregated. This is very similar to private network subnets.
Molter73 pushed a commit that referenced this pull request Jul 24, 2024
ROX_NON_AGGREGATED_NETWORKS can list subnets that should not
be aggregated. This is very similar to private network subnets.
@Molter73 Molter73 mentioned this pull request Jul 24, 2024
5 tasks
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