Skip to content

Commit c408eaa

Browse files
github-actions[bot]romainkomorn-exdatadog
github-actions[bot]
and
romainkomorn-exdatadog
authored
chore(ci_visibility): remove app key requirement for ITR [backport 2.0] (#7122)
Backport 974f580 from #7067 to 2.0. 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 b84dabd commit c408eaa

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
@@ -24,10 +24,7 @@
2424
from ..utils.http import Response
2525
from ..utils.http import get_connection
2626
from .constants import AGENTLESS_API_KEY_HEADER_NAME
27-
from .constants import AGENTLESS_APP_KEY_HEADER_NAME
2827
from .constants import AGENTLESS_DEFAULT_SITE
29-
from .constants import EVP_NEEDS_APP_KEY_HEADER_NAME
30-
from .constants import EVP_NEEDS_APP_KEY_HEADER_VALUE
3128
from .constants import EVP_PROXY_AGENT_BASE_PATH
3229
from .constants import EVP_SUBDOMAIN_HEADER_API_VALUE
3330
from .constants import EVP_SUBDOMAIN_HEADER_NAME
@@ -48,12 +45,11 @@ class CIVisibilityGitClient(object):
4845
def __init__(
4946
self,
5047
api_key,
51-
app_key,
5248
requests_mode=REQUESTS_MODE.AGENTLESS_EVENTS,
5349
base_url="",
5450
):
55-
# type: (str, str, int, str) -> None
56-
self._serializer = CIVisibilityGitClientSerializerV1(api_key, app_key)
51+
# type: (str, int, str) -> None
52+
self._serializer = CIVisibilityGitClientSerializerV1(api_key)
5753
self._worker = None # type: Optional[Process]
5854
self._response = RESPONSE
5955
self._requests_mode = requests_mode
@@ -152,12 +148,10 @@ def _do_request(cls, requests_mode, base_url, endpoint, payload, serializer, hea
152148
url = "{}/repository{}".format(base_url, endpoint)
153149
_headers = {
154150
AGENTLESS_API_KEY_HEADER_NAME: serializer.api_key,
155-
AGENTLESS_APP_KEY_HEADER_NAME: serializer.app_key,
156151
}
157152
if requests_mode == REQUESTS_MODE.EVP_PROXY_EVENTS:
158153
_headers = {
159154
EVP_SUBDOMAIN_HEADER_NAME: EVP_SUBDOMAIN_HEADER_API_VALUE,
160-
EVP_NEEDS_APP_KEY_HEADER_NAME: EVP_NEEDS_APP_KEY_HEADER_VALUE,
161155
}
162156
if headers is not None:
163157
_headers.update(headers)
@@ -213,10 +207,9 @@ def _unshallow_repository(cls, cwd=None):
213207

214208

215209
class CIVisibilityGitClientSerializerV1(object):
216-
def __init__(self, api_key, app_key):
217-
# type: (str, str) -> None
210+
def __init__(self, api_key):
211+
# type: (str) -> None
218212
self.api_key = api_key
219-
self.app_key = app_key
220213

221214
def search_commits_encode(self, repo_url, latest_commits):
222215
# 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
@@ -103,10 +100,6 @@ def __init__(self, tracer=None, config=None, service=None):
103100
# Create a new CI tracer
104101
self.tracer = Tracer(context_provider=CIContextProvider())
105102

106-
self._app_key = os.getenv(
107-
"_CI_DD_APP_KEY",
108-
os.getenv("DD_APP_KEY", os.getenv("DD_APPLICATION_KEY", os.getenv("DATADOG_APPLICATION_KEY"))),
109-
)
110103
self._api_key = os.getenv("_CI_DD_API_KEY", os.getenv("DD_API_KEY"))
111104

112105
self._dd_site = os.getenv("DD_SITE", AGENTLESS_DEFAULT_SITE)
@@ -145,16 +138,12 @@ def __init__(self, tracer=None, config=None, service=None):
145138
self._git_client = None
146139

147140
if ddconfig._ci_visibility_intelligent_testrunner_enabled:
148-
if self._app_key is None:
149-
log.warning("Environment variable DD_APP_KEY not set, so no git metadata will be uploaded.")
150-
elif self._requests_mode == REQUESTS_MODE.TRACES:
141+
if self._requests_mode == REQUESTS_MODE.TRACES:
151142
log.warning("Cannot start git client if mode is not agentless or evp proxy")
152143
else:
153144
if not self._test_skipping_enabled_by_api:
154145
log.warning("Intelligent Test Runner test skipping disabled by API")
155-
self._git_client = CIVisibilityGitClient(
156-
api_key=self._api_key or "", app_key=self._app_key, requests_mode=self._requests_mode
157-
)
146+
self._git_client = CIVisibilityGitClient(api_key=self._api_key or "", requests_mode=self._requests_mode)
158147
try:
159148
from ddtrace.internal.codeowners import Codeowners
160149

@@ -189,17 +178,15 @@ def _check_enabled_features(self):
189178
url = get_trace_url() + EVP_PROXY_AGENT_BASE_PATH + SETTING_ENDPOINT
190179
_headers = {
191180
EVP_SUBDOMAIN_HEADER_NAME: EVP_SUBDOMAIN_HEADER_API_VALUE,
192-
EVP_NEEDS_APP_KEY_HEADER_NAME: EVP_NEEDS_APP_KEY_HEADER_VALUE,
193181
}
194182
log.debug("Making EVP request to agent: url=%s, headers=%s", url, _headers)
195183
elif self._requests_mode == REQUESTS_MODE.AGENTLESS_EVENTS:
196-
if not self._app_key or not self._api_key:
197-
log.debug("Cannot make request to setting endpoint if application key is not set")
184+
if not self._api_key:
185+
log.debug("Cannot make request to setting endpoint if API key is not set")
198186
return False, False
199187
url = "https://api." + self._dd_site + SETTING_ENDPOINT
200188
_headers = {
201189
AGENTLESS_API_KEY_HEADER_NAME: self._api_key,
202-
AGENTLESS_APP_KEY_HEADER_NAME: self._app_key,
203190
"Content-Type": "application/json",
204191
}
205192
else:
@@ -306,14 +293,12 @@ def _fetch_tests_to_skip(self, skipping_mode):
306293

307294
_headers = {
308295
"dd-api-key": self._api_key,
309-
"dd-application-key": self._app_key,
310296
"Content-Type": "application/json",
311297
}
312298
if self._requests_mode == REQUESTS_MODE.EVP_PROXY_EVENTS:
313299
url = get_trace_url() + EVP_PROXY_AGENT_BASE_PATH + SKIPPABLE_ENDPOINT
314300
_headers = {
315301
EVP_SUBDOMAIN_HEADER_NAME: EVP_SUBDOMAIN_HEADER_API_VALUE,
316-
EVP_NEEDS_APP_KEY_HEADER_NAME: EVP_NEEDS_APP_KEY_HEADER_VALUE,
317302
}
318303
elif self._requests_mode == REQUESTS_MODE.AGENTLESS_EVENTS:
319304
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)