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

[APPSEC-6712] Remote configuration config_state #2794

Merged
merged 1 commit into from
Apr 17, 2023

Conversation

GustavoCaso
Copy link
Member

@GustavoCaso GustavoCaso commented Apr 17, 2023

What does this PR do?
Stores and set remote configuration content version. Is that information to populate the client config_state

@lloeki will work on the remaining portion of config_state, which is adding the information if any error occurred during the remote configuration sync

Motivation

Additional Notes

Here are the system test assertions on the expected values for config_states

How to test the change?

CI

@GustavoCaso GustavoCaso requested a review from a team April 17, 2023 10:50
@github-actions github-actions bot added the core Involves Datadog core libraries label Apr 17, 2023
@GustavoCaso GustavoCaso changed the title Remote configuration [APPSEC-6712] Remote configuration config_state Apr 17, 2023
@GustavoCaso GustavoCaso force-pushed the remote-populate-config-states branch from ca8fe4e to 9313881 Compare April 17, 2023 10:57
@GustavoCaso GustavoCaso requested a review from lloeki April 17, 2023 10:58
Copy link
Member

@lloeki lloeki left a comment

Choose a reason for hiding this comment

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

LGTM (once the failure is fixed!)

@@ -48,16 +48,18 @@ class << self
def parse(hash)
length = Integer(hash['length'])
digests = Configuration::DigestList.parse(hash['hashes'])
version = Integer(hash['custom']['v'])
Copy link
Member

Choose a reason for hiding this comment

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

This is failing:

NoMethodError:
       undefined method `[]' for nil:NilClass
     # ./lib/datadog/core/remote/configuration/target.rb:51:in `parse'
     # ./spec/datadog/core/remote/configuration/target_spec.rb:178:in `block (3 levels) in <top (required)>'
     # ./spec/datadog/core/remote/configuration/target_spec.rb:199:in `block (5 levels) in <top (required)>'

Populate config_states
@GustavoCaso GustavoCaso force-pushed the remote-populate-config-states branch from 9313881 to 725210a Compare April 17, 2023 14:37
@GustavoCaso GustavoCaso merged commit 4978bda into master Apr 17, 2023
@GustavoCaso GustavoCaso deleted the remote-populate-config-states branch April 17, 2023 15:24
@github-actions github-actions bot added this to the 1.11.0 milestone Apr 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Involves Datadog core libraries
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants