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

Viper: produce a perms error for config file #13350

Merged
merged 2 commits into from
Sep 7, 2022

Conversation

djmitche
Copy link
Contributor

@djmitche djmitche commented Sep 2, 2022

This updates Viper to a version containing
DataDog/viper#26 which returns a permissions
error if the config file cannot be read due to a permissions problem.
Previously this implementation unconditionally returned a "not found"
error which produced a deceptive message for the user.

Motivation

Better error messages

Possible Drawbacks / Trade-offs

This gets us deeper into our dependency on our fork of Viper.

Describe how to test/QA your changes

Run an agent in a situation where its configuration is inaccessible due to permissions errors (such as chmod 000 /etc/datadog-agent/datadog.yaml). You should see

Error: unable to set up global agent configuration: cannot access the Datadog config file (open /etc/datadog-agent/datadog.yaml: permission denied); try running the command under the same user as the Datadog Agent

do this on both WIndows and linux/mac.

Reviewer's Checklist

  • If known, an appropriate milestone has been selected; otherwise the Triage milestone is set.
  • Use the major_change label if your change either has a major impact on the code base, is impacting multiple teams or is changing important well-established internals of the Agent. This label will be use during QA to make sure each team pay extra attention to the changed behavior. For any customer facing change use a releasenote.
  • A release note has been added or the changelog/no-changelog label has been applied.
  • Changed code has automated tests for its functionality.
  • Adequate QA/testing plan information is provided if the qa/skip-qa label is not applied.
  • At least one team/.. label has been applied, indicating the team(s) that should QA this change.
  • If applicable, docs team has been notified or an issue has been opened on the documentation repo.
  • If applicable, the need-change/operator and need-change/helm labels have been applied.
  • If applicable, the k8s/<min-version> label, indicating the lowest Kubernetes version compatible with this feature.
  • If applicable, the config template has been updated.

@djmitche djmitche requested a review from a team September 2, 2022 17:44
@djmitche djmitche added this to the 7.40.0 milestone Sep 2, 2022
@sgnn7
Copy link
Contributor

sgnn7 commented Sep 2, 2022

Note: Some build failures seem legitimate on the PR

@djmitche
Copy link
Contributor Author

djmitche commented Sep 6, 2022

Looks like this is causing issues reading .hcl files in NPM.

This updates Viper to a version containing
DataDog/viper#26 which returns a permissions
error if the config file cannot be read due to a permissions problem.
Previously this implementation unconditionally returned a "not found"
error which produced a deceptive message for the user.
@djmitche djmitche force-pushed the dustin.mitchell/asc-158-viper-update branch from 229ab09 to 0ce4fa5 Compare September 6, 2022 22:48
@djmitche djmitche requested a review from a team as a code owner September 6, 2022 22:48
@djmitche
Copy link
Contributor Author

djmitche commented Sep 7, 2022

@djmitche
Copy link
Contributor Author

djmitche commented Sep 7, 2022

..and ci/circleci: build_iot_agent and ci/circleci: build_images aren't running because the integration tests aren't. So I'm overriding branch protection.

@djmitche djmitche merged commit 8745213 into main Sep 7, 2022
@djmitche djmitche deleted the dustin.mitchell/asc-158-viper-update branch September 7, 2022 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants