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

Return Azure AD auth token in correct format #15701

Merged
merged 2 commits into from
Aug 29, 2023

Conversation

jmeunier28
Copy link
Contributor

@jmeunier28 jmeunier28 commented Aug 28, 2023

What does this PR do?

This PR fixes a feature, which was added in #15609 to support Azure Managed Identity authentication for Postgresql Flex/Single Server.

The original implementation used a similar approach to sqlserver/pyodbc to set the token, but that was not necessary and resulted in the incorrect token being passed at connection time. I tested this by updating the code on a local install & was able to verify we are seeing all of the expected integration data for our test postgres instance now:

    postgres (14.2.0)
    -----------------
      Instance ID: postgres:f808f645a7eeae1f [OK]
      Configuration Source: file:/etc/datadog-agent/conf.d/postgres.d/conf.yaml
      Total Runs: 3
      Metric Samples: Last Run: 466, Total: 1,291
      Events: Last Run: 0, Total: 0
      Database Monitoring Activity Samples: Last Run: 1, Total: 3
      Database Monitoring Metadata Samples: Last Run: 1, Total: 1
      Database Monitoring Query Metrics: Last Run: 1, Total: 3
      Database Monitoring Query Samples: Last Run: 21, Total: 77
      Service Checks: Last Run: 1, Total: 3
      Average Execution Time : 116ms
      Last Execution Date : 2023-08-28 19:42:42 UTC (1693251762000)
      Last Successful Execution Date : 2023-08-28 19:42:42 UTC (1693251762000)
      metadata:
        resolved_hostname: dbm-postgres13-flex-server.postgres.database.azure.com
        version.major: 13
        version.minor: 0
        version.patch: 0
        version.raw: 13.0.0
        version.scheme: semver

Motivation

This feature was implemented as part of an effort to support more secure ways to authenticate to databases in the cloud. As we previously added support for IAM, this allows azure users to connect using managed authentication. This is a highly requested customer feature.

@jmeunier28 jmeunier28 requested review from a team as code owners August 28, 2023 20:01
@jmeunier28 jmeunier28 added this to the 7.48.0 milestone Aug 28, 2023
@codecov
Copy link

codecov bot commented Aug 28, 2023

Codecov Report

Merging #15701 (197ae22) into master (441957c) will increase coverage by 0.00%.
The diff coverage is 0.00%.

Flag Coverage Δ
postgres 91.88% <0.00%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

@github-actions
Copy link

Test Results

     12 files       12 suites   15m 0s ⏱️
   228 tests    226 ✔️   2 💤 0
1 374 runs  1 326 ✔️ 48 💤 0

Results for commit 197ae22.

Copy link
Contributor

@cswatt cswatt left a comment

Choose a reason for hiding this comment

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

changelog approved by docs

@jmeunier28 jmeunier28 merged commit a80db1a into master Aug 29, 2023
39 checks passed
@jmeunier28 jmeunier28 deleted the jmeunier/agent-fix-azure-auth branch August 29, 2023 12:47
vivek-datadog pushed a commit that referenced this pull request Aug 29, 2023
* Return Azure AD auth token in correct format

* changelog
vivek-datadog added a commit that referenced this pull request Aug 29, 2023
* Return Azure AD auth token in correct format (#15701)

* [Release] Bumped postgres version to 14.2.1

* [Release] Update metadata

---------

Co-authored-by: JoJo <jojo.meunier@datadoghq.com>
jmeunier28 added a commit that referenced this pull request Sep 18, 2023
* Return Azure AD auth token in correct format

* changelog
jmeunier28 added a commit that referenced this pull request Sep 18, 2023
* Return Azure AD auth token in correct format

* changelog
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.

4 participants