Skip to content

Commit 090603d

Browse files
github-actions[bot]romainkomorn-exdatadog
github-actions[bot]
and
romainkomorn-exdatadog
authored
chore(ci_visibility): remove app key requirement for ITR [backport 1.20] (#7372)
Backport 974f580 from #7067 to 1.20. The Datadog Intelligent Test Runner API endpoints are removing the need for an application key to be supplied when getting skippable tests and settings. There is no associated release note even though this is arguably a breaking change because it is still in beta. ## Checklist - [x] Change(s) are motivated and described in the PR description. - [x] Testing strategy is described if automated tests are not included in the PR. - [x] Risk is outlined (performance impact, potential for breakage, maintainability, etc). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] [Library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) are followed. If no release note is required, add label `changelog/no-changelog`. - [x] Documentation is included (in-code, generated user docs, [public corp docs](https://github.com/DataDog/documentation/)). - [x] Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] Title is accurate. - [x] No unnecessary changes are introduced. - [x] Description motivates each change. - [x] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes unless absolutely necessary. - [x] Testing strategy adequately addresses listed risk(s). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] Release note makes sense to a user of the library. - [x] Reviewer has explicitly acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment. - [x] Backport labels are set in a manner that is consistent with the [release branch maintenance policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting) - [x] If this PR touches code that signs or publishes builds or packages, or handles credentials of any kind, I've requested a review from `@DataDog/security-design-and-guidance`. - [x] This PR doesn't touch any of that. Co-authored-by: Romain Komorn <[email protected]>
1 parent 817003a commit 090603d

File tree

3 files changed

+17
-70
lines changed

3 files changed

+17
-70
lines changed

‎ddtrace/internal/ci_visibility/git_client.py‎

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,7 @@
2626
from ..utils.http import Response
2727
from ..utils.http import get_connection
2828
from .constants import AGENTLESS_API_KEY_HEADER_NAME
29-
from .constants import AGENTLESS_APP_KEY_HEADER_NAME
3029
from .constants import AGENTLESS_DEFAULT_SITE
31-
from .constants import EVP_NEEDS_APP_KEY_HEADER_NAME
32-
from .constants import EVP_NEEDS_APP_KEY_HEADER_VALUE
3330
from .constants import EVP_PROXY_AGENT_BASE_PATH
3431
from .constants import EVP_SUBDOMAIN_HEADER_API_VALUE
3532
from .constants import EVP_SUBDOMAIN_HEADER_NAME
@@ -50,12 +47,11 @@ class CIVisibilityGitClient(object):
5047
def __init__(
5148
self,
5249
api_key,
53-
app_key,
5450
requests_mode=REQUESTS_MODE.AGENTLESS_EVENTS,
5551
base_url="",
5652
):
57-
# type: (str, str, int, str) -> None
58-
self._serializer = CIVisibilityGitClientSerializerV1(api_key, app_key)
53+
# type: (str, int, str) -> None
54+
self._serializer = CIVisibilityGitClientSerializerV1(api_key)
5955
self._worker = None # type: Optional[Process]
6056
self._response = RESPONSE
6157
self._requests_mode = requests_mode
@@ -155,12 +151,10 @@ def _do_request(cls, requests_mode, base_url, endpoint, payload, serializer, hea
155151
url = "{}/repository{}".format(base_url, endpoint)
156152
_headers = {
157153
AGENTLESS_API_KEY_HEADER_NAME: serializer.api_key,
158-
AGENTLESS_APP_KEY_HEADER_NAME: serializer.app_key,
159154
}
160155
if requests_mode == REQUESTS_MODE.EVP_PROXY_EVENTS:
161156
_headers = {
162157
EVP_SUBDOMAIN_HEADER_NAME: EVP_SUBDOMAIN_HEADER_API_VALUE,
163-
EVP_NEEDS_APP_KEY_HEADER_NAME: EVP_NEEDS_APP_KEY_HEADER_VALUE,
164158
}
165159
if headers is not None:
166160
_headers.update(headers)
@@ -256,10 +250,9 @@ def _unshallow_repository_to_upstream(cls, remote, cwd=None):
256250

257251

258252
class CIVisibilityGitClientSerializerV1(object):
259-
def __init__(self, api_key, app_key):
260-
# type: (str, str) -> None
253+
def __init__(self, api_key):
254+
# type: (str) -> None
261255
self.api_key = api_key
262-
self.app_key = app_key
263256

264257
def search_commits_encode(self, repo_url, latest_commits):
265258
# type: (str, list[str]) -> str

‎ddtrace/internal/ci_visibility/recorder.py‎

Lines changed: 4 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,7 @@
2626

2727
from .. import agent
2828
from .constants import AGENTLESS_API_KEY_HEADER_NAME
29-
from .constants import AGENTLESS_APP_KEY_HEADER_NAME
3029
from .constants import AGENTLESS_DEFAULT_SITE
31-
from .constants import EVP_NEEDS_APP_KEY_HEADER_NAME
32-
from .constants import EVP_NEEDS_APP_KEY_HEADER_VALUE
3330
from .constants import EVP_PROXY_AGENT_BASE_PATH
3431
from .constants import EVP_SUBDOMAIN_HEADER_API_VALUE
3532
from .constants import EVP_SUBDOMAIN_HEADER_EVENT_VALUE
@@ -106,10 +103,6 @@ def __init__(self, tracer=None, config=None, service=None):
106103
else:
107104
self.tracer = Tracer()
108105

109-
self._app_key = os.getenv(
110-
"_CI_DD_APP_KEY",
111-
os.getenv("DD_APP_KEY", os.getenv("DD_APPLICATION_KEY", os.getenv("DATADOG_APPLICATION_KEY"))),
112-
)
113106
self._api_key = os.getenv("_CI_DD_API_KEY", os.getenv("DD_API_KEY"))
114107

115108
self._dd_site = os.getenv("DD_SITE", AGENTLESS_DEFAULT_SITE)
@@ -148,16 +141,12 @@ def __init__(self, tracer=None, config=None, service=None):
148141
self._git_client = None
149142

150143
if ddconfig._ci_visibility_intelligent_testrunner_enabled:
151-
if self._app_key is None:
152-
log.warning("Environment variable DD_APP_KEY not set, so no git metadata will be uploaded.")
153-
elif self._requests_mode == REQUESTS_MODE.TRACES:
144+
if self._requests_mode == REQUESTS_MODE.TRACES:
154145
log.warning("Cannot start git client if mode is not agentless or evp proxy")
155146
else:
156147
if not self._test_skipping_enabled_by_api:
157148
log.warning("Intelligent Test Runner test skipping disabled by API")
158-
self._git_client = CIVisibilityGitClient(
159-
api_key=self._api_key or "", app_key=self._app_key, requests_mode=self._requests_mode
160-
)
149+
self._git_client = CIVisibilityGitClient(api_key=self._api_key or "", requests_mode=self._requests_mode)
161150
try:
162151
from ddtrace.internal.codeowners import Codeowners
163152

@@ -192,17 +181,15 @@ def _check_enabled_features(self):
192181
url = get_trace_url() + EVP_PROXY_AGENT_BASE_PATH + SETTING_ENDPOINT
193182
_headers = {
194183
EVP_SUBDOMAIN_HEADER_NAME: EVP_SUBDOMAIN_HEADER_API_VALUE,
195-
EVP_NEEDS_APP_KEY_HEADER_NAME: EVP_NEEDS_APP_KEY_HEADER_VALUE,
196184
}
197185
log.debug("Making EVP request to agent: url=%s, headers=%s", url, _headers)
198186
elif self._requests_mode == REQUESTS_MODE.AGENTLESS_EVENTS:
199-
if not self._app_key or not self._api_key:
200-
log.debug("Cannot make request to setting endpoint if application key is not set")
187+
if not self._api_key:
188+
log.debug("Cannot make request to setting endpoint if API key is not set")
201189
return False, False
202190
url = "https://api." + self._dd_site + SETTING_ENDPOINT
203191
_headers = {
204192
AGENTLESS_API_KEY_HEADER_NAME: self._api_key,
205-
AGENTLESS_APP_KEY_HEADER_NAME: self._app_key,
206193
"Content-Type": "application/json",
207194
}
208195
else:
@@ -309,14 +296,12 @@ def _fetch_tests_to_skip(self, skipping_mode):
309296

310297
_headers = {
311298
"dd-api-key": self._api_key,
312-
"dd-application-key": self._app_key,
313299
"Content-Type": "application/json",
314300
}
315301
if self._requests_mode == REQUESTS_MODE.EVP_PROXY_EVENTS:
316302
url = get_trace_url() + EVP_PROXY_AGENT_BASE_PATH + SKIPPABLE_ENDPOINT
317303
_headers = {
318304
EVP_SUBDOMAIN_HEADER_NAME: EVP_SUBDOMAIN_HEADER_API_VALUE,
319-
EVP_NEEDS_APP_KEY_HEADER_NAME: EVP_NEEDS_APP_KEY_HEADER_VALUE,
320305
}
321306
elif self._requests_mode == REQUESTS_MODE.AGENTLESS_EVENTS:
322307
url = "https://api." + self._dd_site + SKIPPABLE_ENDPOINT

‎tests/ci_visibility/test_ci_visibility.py‎

Lines changed: 9 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -315,15 +315,15 @@ def test_git_client_get_latest_commits(git_repo):
315315
def test_git_client_search_commits():
316316
remote_url = "[email protected]:test-repo-url.git"
317317
latest_commits = [TEST_SHA]
318-
serializer = CIVisibilityGitClientSerializerV1("foo", "bar")
318+
serializer = CIVisibilityGitClientSerializerV1("foo")
319319
backend_commits = CIVisibilityGitClient._search_commits(
320320
REQUESTS_MODE.AGENTLESS_EVENTS, "", remote_url, latest_commits, serializer, DUMMY_RESPONSE
321321
)
322322
assert latest_commits[0] in backend_commits
323323

324324

325325
def test_get_client_do_request_agentless_headers():
326-
serializer = CIVisibilityGitClientSerializerV1("foo", "bar")
326+
serializer = CIVisibilityGitClientSerializerV1("foo")
327327
response = mock.MagicMock()
328328
response.status = 200
329329

@@ -334,13 +334,11 @@ def test_get_client_do_request_agentless_headers():
334334
REQUESTS_MODE.AGENTLESS_EVENTS, "http://base_url", "/endpoint", "payload", serializer, {}
335335
)
336336

337-
_request.assert_called_once_with(
338-
"POST", "http://base_url/repository/endpoint", "payload", {"dd-api-key": "foo", "dd-application-key": "bar"}
339-
)
337+
_request.assert_called_once_with("POST", "http://base_url/repository/endpoint", "payload", {"dd-api-key": "foo"})
340338

341339

342340
def test_get_client_do_request_evp_proxy_headers():
343-
serializer = CIVisibilityGitClientSerializerV1("foo", "bar")
341+
serializer = CIVisibilityGitClientSerializerV1("foo")
344342
response = mock.MagicMock()
345343
response.status = 200
346344

@@ -355,7 +353,7 @@ def test_get_client_do_request_evp_proxy_headers():
355353
"POST",
356354
"http://base_url/repository/endpoint",
357355
"payload",
358-
{"X-Datadog-EVP-Subdomain": "api", "X-Datadog-NeedsAppKey": "true"},
356+
{"X-Datadog-EVP-Subdomain": "api"},
359357
)
360358

361359

@@ -413,7 +411,7 @@ def test_git_client_build_packfiles_temp_dir_value_error(_temp_dir_mock, git_rep
413411

414412

415413
def test_git_client_upload_packfiles(git_repo):
416-
serializer = CIVisibilityGitClientSerializerV1("foo", "bar")
414+
serializer = CIVisibilityGitClientSerializerV1("foo")
417415
remote_url = "[email protected]:test-repo-url.git"
418416
with CIVisibilityGitClient._build_packfiles("%s\n" % TEST_SHA, cwd=git_repo) as packfiles_path:
419417
with mock.patch("ddtrace.internal.ci_visibility.git_client.CIVisibilityGitClient._do_request") as dr:
@@ -431,7 +429,7 @@ def test_git_client_upload_packfiles(git_repo):
431429

432430

433431
def test_git_do_request_agentless(git_repo):
434-
mock_serializer = CIVisibilityGitClientSerializerV1("fakeapikey", "fakeappkey")
432+
mock_serializer = CIVisibilityGitClientSerializerV1("fakeapikey")
435433
response = mock.MagicMock()
436434
setattr(response, "status", 200) # noqa: B010
437435

@@ -458,14 +456,13 @@ def test_git_do_request_agentless(git_repo):
458456
'{"payload": "payload"}',
459457
{
460458
"dd-api-key": "fakeapikey",
461-
"dd-application-key": "fakeappkey",
462459
"mock_header_name": "mock_header_value",
463460
},
464461
)
465462

466463

467464
def test_git_do_request_evp(git_repo):
468-
mock_serializer = CIVisibilityGitClientSerializerV1("foo", "bar")
465+
mock_serializer = CIVisibilityGitClientSerializerV1("foo")
469466
response = mock.MagicMock()
470467
setattr(response, "status", 200) # noqa: B010
471468

@@ -491,7 +488,6 @@ def test_git_do_request_evp(git_repo):
491488
"base_url/repositoryendpoint",
492489
'{"payload": "payload"}',
493490
{
494-
"X-Datadog-NeedsAppKey": "true",
495491
"X-Datadog-EVP-Subdomain": "api",
496492
"mock_header_name": "mock_header_value",
497493
},
@@ -615,7 +611,6 @@ def test_civisibility_check_enabled_features_agentless_do_request_called_correct
615611
mock_civisibilty._requests_mode = REQUESTS_MODE.AGENTLESS_EVENTS
616612
mock_civisibilty._service = "service"
617613
mock_civisibilty._api_key = "myfakeapikey"
618-
mock_civisibilty._app_key = "myfakeappkey"
619614
mock_civisibilty._dd_site = "datad0g.com"
620615
mock_civisibilty._tags = {
621616
ci.git.REPOSITORY_URL: "my_repo_url",
@@ -633,7 +628,6 @@ def test_civisibility_check_enabled_features_agentless_do_request_called_correct
633628
assert do_request_call_args[1] == "https://api.datad0g.com/api/v2/libraries/tests/services/setting"
634629
assert do_request_call_args[3] == {
635630
"dd-api-key": "myfakeapikey",
636-
"dd-application-key": "myfakeappkey",
637631
"Content-Type": "application/json",
638632
}
639633
assert do_request_payload == {
@@ -686,7 +680,6 @@ def test_civisibility_check_enabled_features_evp_do_request_called_correctly():
686680
)
687681
assert do_request_call_args[3] == {
688682
"X-Datadog-EVP-Subdomain": "api",
689-
"X-Datadog-NeedsAppKey": "true",
690683
}
691684
assert do_request_payload == {
692685
"data": {
@@ -704,26 +697,6 @@ def test_civisibility_check_enabled_features_evp_do_request_called_correctly():
704697
assert enabled_features == (True, True)
705698

706699

707-
@mock.patch("ddtrace.internal.ci_visibility.recorder._do_request")
708-
def test_civisibility_check_enabled_features_no_app_key_request_not_called(_do_request):
709-
with override_env(
710-
dict(
711-
DD_API_KEY="foo.bar",
712-
DD_CIVISIBILITY_AGENTLESS_URL="https://foo.bar",
713-
DD_CIVISIBILITY_AGENTLESS_ENABLED="1",
714-
DD_CIVISIBILITY_ITR_ENABLED="1",
715-
)
716-
):
717-
ddtrace.internal.ci_visibility.writer.config = ddtrace.settings.Config()
718-
ddtrace.internal.ci_visibility.recorder.ddconfig = ddtrace.settings.Config()
719-
CIVisibility.enable()
720-
721-
_do_request.assert_not_called()
722-
assert CIVisibility._instance._code_coverage_enabled_by_api is False
723-
assert CIVisibility._instance._test_skipping_enabled_by_api is False
724-
CIVisibility.disable()
725-
726-
727700
@mock.patch("ddtrace.internal.ci_visibility.recorder._do_request")
728701
def test_civisibility_check_enabled_features_itr_disabled_request_not_called(_do_request):
729702
with override_env(
@@ -755,7 +728,6 @@ def test_civisibility_check_enabled_features_itr_enabled_request_called(_do_requ
755728
with override_env(
756729
dict(
757730
DD_API_KEY="foo.bar",
758-
DD_APP_KEY="foobar.baz",
759731
DD_CIVISIBILITY_AGENTLESS_URL="https://foo.bar",
760732
DD_CIVISIBILITY_AGENTLESS_ENABLED="1",
761733
DD_CIVISIBILITY_ITR_ENABLED="1",
@@ -791,7 +763,7 @@ def test_civisibility_check_enabled_features_itr_enabled_request_called(_do_requ
791763
}
792764
}
793765
),
794-
{"dd-api-key": "foo.bar", "dd-application-key": "foobar.baz", "Content-Type": "application/json"},
766+
{"dd-api-key": "foo.bar", "Content-Type": "application/json"},
795767
)
796768
assert CIVisibility._instance._code_coverage_enabled_by_api is True
797769
assert CIVisibility._instance._test_skipping_enabled_by_api is True
@@ -810,7 +782,6 @@ def test_civisibility_check_enabled_features_itr_enabled_errors_not_found(_do_re
810782
with override_env(
811783
dict(
812784
DD_API_KEY="foo.bar",
813-
DD_APP_KEY="foobar.baz",
814785
DD_CIVISIBILITY_AGENTLESS_URL="https://foo.bar",
815786
DD_CIVISIBILITY_AGENTLESS_ENABLED="1",
816787
DD_CIVISIBILITY_ITR_ENABLED="1",
@@ -838,7 +809,6 @@ def test_civisibility_check_enabled_features_itr_enabled_404_response(_do_reques
838809
with override_env(
839810
dict(
840811
DD_API_KEY="foo.bar",
841-
DD_APP_KEY="foobar.baz",
842812
DD_CIVISIBILITY_AGENTLESS_URL="https://foo.bar",
843813
DD_CIVISIBILITY_AGENTLESS_ENABLED="1",
844814
DD_CIVISIBILITY_ITR_ENABLED="1",
@@ -868,7 +838,6 @@ def test_civisibility_check_enabled_features_itr_enabled_malformed_response(_do_
868838
with override_env(
869839
dict(
870840
DD_API_KEY="foo.bar",
871-
DD_APP_KEY="foobar.baz",
872841
DD_CIVISIBILITY_AGENTLESS_URL="https://foo.bar",
873842
DD_CIVISIBILITY_AGENTLESS_ENABLED="1",
874843
DD_CIVISIBILITY_ITR_ENABLED="1",

0 commit comments

Comments
 (0)