Skip to content

feat(shield): improve regions handling logic to allow override region specific settings without setting region to custom #2216

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

Merged
merged 2 commits into from
Apr 10, 2025

Conversation

maratsal
Copy link
Contributor

What this PR does / why we need it:

improve regions handling logic to allow override region specific settings without setting region to custom. Currently if we want to use alternative collector settings we have to use custom region and specify each of the following values:

sysdig_endpoint:
  region: custom
  api_url: https://www.example.com
  collector:
    host: example.com
    port: 6443

Many customers want to keep using default region settings and update only collector endpoints and use config like below:

sysdig_endpoint:
  region: us2
  collector:
    host: alt-example.com
    port: 443

This PR is addressing that functionality.

Checklist

  • Title of the PR starts with type and scope, (e.g. feat(agent,node-analyzer,sysdig-deploy):)
  • Chart Version bumped for the respective charts
  • Variables are documented in the README.md (or README.tpl in some charts)
  • Check GithubAction checks (like lint) to avoid merge-check stoppers
  • All test files are added in the tests folder of their respective chart and have a "_test" suffix

@maratsal maratsal requested a review from a team as a code owner April 10, 2025 13:22
@maratsal maratsal force-pushed the feat-improve-regions-handling branch from f1faef4 to 954d350 Compare April 10, 2025 13:26
@@ -180,16 +180,16 @@

{{- define "common.collector_endpoint" }}
{{- $regions := fromYaml (include "common.regions" .) }}
{{- if ne "custom" .Values.sysdig_endpoint.region }}
{{- get (get $regions .Values.sysdig_endpoint.region) "collector_endpoint" }}
{{- if and (ne "custom" .Values.sysdig_endpoint.region) (not .Values.sysdig_endpoint.collector.host) }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

@maratsal - If I understand the intent of this PR I think what the intent of this change is, is to use sysdig_endpoint.collector.host any time it is specified, correct? If that's true, we could simplify the boolean logic here.

If my understanding is correct, you could apply the simplification to the other areas you've modified as well.

cc @AlbertoBarba

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe, if you can share what you are thinking of - my brain always comes back to couple of checks anyways :D

Copy link
Collaborator

Choose a reason for hiding this comment

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

Well, if I'm not mistaken, this change is intended to use sysdig_endpoint.collector.host whether the region field is custom or some valid region, correct? If that's the case, you can simplify the checks here to simply see if sysdig_endpoint.collector.host is non-null.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated logic.

@maratsal maratsal force-pushed the feat-improve-regions-handling branch 2 times, most recently from 00b40a3 to cedaf2d Compare April 10, 2025 19:17
@maratsal maratsal force-pushed the feat-improve-regions-handling branch from cedaf2d to 1f36699 Compare April 10, 2025 19:19
@maratsal maratsal enabled auto-merge (squash) April 10, 2025 19:30
@maratsal maratsal merged commit 579aa4c into main Apr 10, 2025
4 checks passed
@maratsal maratsal deleted the feat-improve-regions-handling branch April 10, 2025 19:35
maratsal added a commit that referenced this pull request Apr 10, 2025
…e region specific settings without setting region to custom (#2216)"

This reverts commit 579aa4c.
maratsal added a commit that referenced this pull request Apr 10, 2025
… specific settings without setting region to custom (#2216)
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