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

CNS-2791: Add a remote configurator that pulls changes from CB Console and applies them to the CR #184

Merged
merged 69 commits into from
Sep 20, 2023

Conversation

ltsonov-cb
Copy link
Contributor

@ltsonov-cb ltsonov-cb commented Sep 12, 2023

This MR adds a secondary "loop" within the operator, in addition to the main one that is responsible to maintain the agent.

Structure:

  • A new controller that is only responsible to run the "reconfiguration" code every X seconds/minutes. It will also do exponential backoff if that fails to avoid clusters swarming us with requests for pending changes.
  • A "configurator" that handles the main logic - getting the CR, reading and pending changes, changing the CR, etc
  • Two helpers that validate a change (i.e. is it even OK to do it? - this should also be done on the backend) and apply it. Makes testing easier
  • The feature is still incomplete. The API for changes is "mocked" and returns random changes instead. This is so a developer can try it out locally. It will basically reconfigure the CR every N seconds right now with random (valid) values.
  • Due to the above bullet, the feature is currently disabled by default (requires env var to run). But in the final version it will be enabled by default and controlled by the CR. The latter is implemented so we only have to remove the env var before release.

The major changes are under remote_configuration package so I'd recommend starting the review from there and going Controller->Configurator->Change_applier.

Other changes:

  • ApiGateway and AccessToken were required in both places so I extracted those from the controller's code
  • The cluster identifier is currently propagated to ApiGateway calls. But we should refactor this code a bit since other cluster ID values (group+name) are part of the apigateway itself and it's hard to follow.

Separate MR will handle:

  • Implementing the ConfigChanges API client
  • More docs

BenRub and others added 30 commits August 30, 2023 12:28
…parate helper function to make testing easier
remote_configuration/change_applier.go Outdated Show resolved Hide resolved
api/v1/cbcontainersagent_types.go Show resolved Hide resolved
"testing"
)

func TestFeatureTogglesAreAppliedCorrectly(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this test tried to save code by wrapping the tests with appliedChangeTest, iterating them and run them, but since the actual test is only:

remote_configuration.ApplyConfigChangeToCR(testCase.change, &testCase.initialCR)
testCase.assertFinalCR(t, testCase.initialCR)

I think this wrapping and "code saving" didn't save a lot of code, but did made the test more messy and harder to read and understand.

I think simple 4 t.Run tests with the 4 test cases will be clearer and probably will resolve with close code lines

@ltsonov-cb ltsonov-cb merged commit 82c2bff into develop Sep 20, 2023
2 checks passed
BenRub added a commit that referenced this pull request Oct 19, 2023
…e and applies them to the CR (#184)

* Update README.md

* Update README.md

* Fix charts for main to be idetical to latest release v6.0.1

* Update README.md

* Adding labels field to charts

* Adding company code secret template to charts

* Adding company code secret template to charts fix

* Add documentation for secrets with helm

* config applier skeleton

* More work on the configuration applier loop.

* Happy path test for config_applier

* Pass context to API functions as they'll do IO

* More tests

* Send Failed status to backend when failing to list or update CR

* Treat missing CR when a change is pending as error

* Move "scheduling" logic to a separate struct for config applier

* Use a DummyAPI implementation

* Small refactoring

* Add test for feature toggles

* Add tests for the version field. Split the modification logic in a separate helper function to make testing easier

* Sort the change slice in case there are multiple pending changes

* Clearing TODOs, small fixes

* Removing more TODOs

* Turn off configurator by default and use env var to enable

* Renaming things

* Refactor tests a bit and remove more TODOs

* Change the configurator to read the CR first , so it can extract the cluster name

* Validate change against sensor capabilities

* Validate operator and agent version compatibility

* Tests for positive validation path

* Bring the Validate and Apply change funcs under 1 struct and validate before applying

* Adding change validation to the configuration

* Change validator interface and add fetcher to download metadata from the API

* Add auth_provider and adjust test to match

* Create AccessTokenProvider in main.go

* Extract the ApiGateway from processors.* and use it in configurator as well.

* Go back to the previous version as it looked simpler

* Fixing some tests and main.go

* Change the agent_processor tests to match the new func. Remove a test that was not deterministic

* Add clusterIdentifier parameter to GetConfigurationChanges

* Add clusterID to the status update model call

* Add secret detection to the changer

* Add secret detection to the validation

* Minor TODOs

* Some missing test cases for validation

* More minor changes

* Add sensor metadata implementation. Add dummy config changes implementation.

* Move data classes around

* Removing TODOs

* Change the feature toggle to be part of the CR

* Change errors to have system info and user info for debugging vs user context in the UI

* Small refactor

* Change dummy struct to just a func

* Bump timeout a bit

* Remove controller_test.go

* Add back the env var during development until the feature is completed (so it is not ON by default when testing)

* Add remote_configuration to docker build

* Fix not wrapped error

* Fix missing generate run

* Fix sensors path and response

* Increase iteration sleep time

* Fix helm indentation

* Change the version reset to be more readable

* Move the remote_configuration under cbcontainers and controllers

* Added godoc

---------

Co-authored-by: BenRub <[email protected]>
Co-authored-by: benrub <[email protected]>
meori pushed a commit that referenced this pull request Oct 22, 2023
…e and applies them to the CR (#184)

* Update README.md

* Update README.md

* Fix charts for main to be idetical to latest release v6.0.1

* Update README.md

* Adding labels field to charts

* Adding company code secret template to charts

* Adding company code secret template to charts fix

* Add documentation for secrets with helm

* config applier skeleton

* More work on the configuration applier loop.

* Happy path test for config_applier

* Pass context to API functions as they'll do IO

* More tests

* Send Failed status to backend when failing to list or update CR

* Treat missing CR when a change is pending as error

* Move "scheduling" logic to a separate struct for config applier

* Use a DummyAPI implementation

* Small refactoring

* Add test for feature toggles

* Add tests for the version field. Split the modification logic in a separate helper function to make testing easier

* Sort the change slice in case there are multiple pending changes

* Clearing TODOs, small fixes

* Removing more TODOs

* Turn off configurator by default and use env var to enable

* Renaming things

* Refactor tests a bit and remove more TODOs

* Change the configurator to read the CR first , so it can extract the cluster name

* Validate change against sensor capabilities

* Validate operator and agent version compatibility

* Tests for positive validation path

* Bring the Validate and Apply change funcs under 1 struct and validate before applying

* Adding change validation to the configuration

* Change validator interface and add fetcher to download metadata from the API

* Add auth_provider and adjust test to match

* Create AccessTokenProvider in main.go

* Extract the ApiGateway from processors.* and use it in configurator as well.

* Go back to the previous version as it looked simpler

* Fixing some tests and main.go

* Change the agent_processor tests to match the new func. Remove a test that was not deterministic

* Add clusterIdentifier parameter to GetConfigurationChanges

* Add clusterID to the status update model call

* Add secret detection to the changer

* Add secret detection to the validation

* Minor TODOs

* Some missing test cases for validation

* More minor changes

* Add sensor metadata implementation. Add dummy config changes implementation.

* Move data classes around

* Removing TODOs

* Change the feature toggle to be part of the CR

* Change errors to have system info and user info for debugging vs user context in the UI

* Small refactor

* Change dummy struct to just a func

* Bump timeout a bit

* Remove controller_test.go

* Add back the env var during development until the feature is completed (so it is not ON by default when testing)

* Add remote_configuration to docker build

* Fix not wrapped error

* Fix missing generate run

* Fix sensors path and response

* Increase iteration sleep time

* Fix helm indentation

* Change the version reset to be more readable

* Move the remote_configuration under cbcontainers and controllers

* Added godoc

---------

Co-authored-by: BenRub <[email protected]>
Co-authored-by: benrub <[email protected]>
BenRub added a commit that referenced this pull request Oct 31, 2023
…e and applies them to the CR (#184)

* Update README.md

* Update README.md

* Fix charts for main to be idetical to latest release v6.0.1

* Update README.md

* Adding labels field to charts

* Adding company code secret template to charts

* Adding company code secret template to charts fix

* Add documentation for secrets with helm

* config applier skeleton

* More work on the configuration applier loop.

* Happy path test for config_applier

* Pass context to API functions as they'll do IO

* More tests

* Send Failed status to backend when failing to list or update CR

* Treat missing CR when a change is pending as error

* Move "scheduling" logic to a separate struct for config applier

* Use a DummyAPI implementation

* Small refactoring

* Add test for feature toggles

* Add tests for the version field. Split the modification logic in a separate helper function to make testing easier

* Sort the change slice in case there are multiple pending changes

* Clearing TODOs, small fixes

* Removing more TODOs

* Turn off configurator by default and use env var to enable

* Renaming things

* Refactor tests a bit and remove more TODOs

* Change the configurator to read the CR first , so it can extract the cluster name

* Validate change against sensor capabilities

* Validate operator and agent version compatibility

* Tests for positive validation path

* Bring the Validate and Apply change funcs under 1 struct and validate before applying

* Adding change validation to the configuration

* Change validator interface and add fetcher to download metadata from the API

* Add auth_provider and adjust test to match

* Create AccessTokenProvider in main.go

* Extract the ApiGateway from processors.* and use it in configurator as well.

* Go back to the previous version as it looked simpler

* Fixing some tests and main.go

* Change the agent_processor tests to match the new func. Remove a test that was not deterministic

* Add clusterIdentifier parameter to GetConfigurationChanges

* Add clusterID to the status update model call

* Add secret detection to the changer

* Add secret detection to the validation

* Minor TODOs

* Some missing test cases for validation

* More minor changes

* Add sensor metadata implementation. Add dummy config changes implementation.

* Move data classes around

* Removing TODOs

* Change the feature toggle to be part of the CR

* Change errors to have system info and user info for debugging vs user context in the UI

* Small refactor

* Change dummy struct to just a func

* Bump timeout a bit

* Remove controller_test.go

* Add back the env var during development until the feature is completed (so it is not ON by default when testing)

* Add remote_configuration to docker build

* Fix not wrapped error

* Fix missing generate run

* Fix sensors path and response

* Increase iteration sleep time

* Fix helm indentation

* Change the version reset to be more readable

* Move the remote_configuration under cbcontainers and controllers

* Added godoc

---------

Co-authored-by: BenRub <[email protected]>
Co-authored-by: benrub <[email protected]>
@ltsonov-cb ltsonov-cb deleted the CNS-2791-remote-configuration-applier branch November 8, 2023 21:08
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.

3 participants