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

Add better support for the new RC API #2757

Merged
merged 19 commits into from
Jul 18, 2024

Conversation

christophe-papazian
Copy link
Contributor

@christophe-papazian christophe-papazian commented Jul 16, 2024

Motivation

We need to be able to

  • delete RC files (for example, one click deactivation is working like that)
  • keep track of RC states between tests of a same scenario

Remote config works like a permanent state represented as a set of files (configurations) that we can modify or delete. This PR changes the API to reflect that.

Please remember that each scenario using remote config tests share a single state among all tests of the scenario. No assumption should be made on the tests order, so each setup should set the desired state with reset and set_config at first (at test or class level).

Changes

  • refactor the api by exposing a single rc_state object
  • rename add_client_config to set_config
  • add del_config and reset to delete RC files in the current state
  • automatically keep track of config and files version to ensure the next send is taken into account as expected
  • allow empty list of files to be sent to reset everything and wait for confirmation (using global version watching)
  • update documentation
  • update all previous tests using the new API
  • Add a more complex test enabling and disabling ASM 4 times in a row, waiting and testing at each step.
  • Update all manifests

APPSEC-54105

Workflow

  1. ⚠️ Create your PR as draft ⚠️
  2. Work on you PR until the CI passes (if something not related to your task is failing, you can ignore it)
  3. Mark it as ready for review
    • Test logic is modified? -> Get a review from RFC owner. We're working on refining the codeowners file quickly.
    • Framework is modified, or non obvious usage of it -> get a review from R&P team

🚀 Once your PR is reviewed, you can merge it!

🛟 #apm-shared-testing 🛟

Reviewer checklist

  • If PR title starts with [<language>], double-check that only <language> is impacted by the change
  • No system-tests internal is modified. Otherwise, I have the approval from R&P team
  • CI is green, or failing jobs are not related to this change (and you are 100% sure about this statement)
  • A docker base image is modified?
    • the relevant build-XXX-image label is present
  • A scenario is added (or removed)?

@christophe-papazian christophe-papazian added the enhancement New feature or request label Jul 16, 2024
Copy link
Collaborator

@cbeauchesne cbeauchesne left a comment

Choose a reason for hiding this comment

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

Reviewed by zoom, some changes to come :)

Copy link
Collaborator

@cbeauchesne cbeauchesne left a comment

Choose a reason for hiding this comment

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

Small requests change, great work !

@christophe-papazian christophe-papazian merged commit e4484e6 into main Jul 18, 2024
326 of 328 checks passed
@christophe-papazian christophe-papazian deleted the christophe-papazian/upgrade_rc_api branch July 18, 2024 13:57
@cbeauchesne
Copy link
Collaborator

❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants