-
Notifications
You must be signed in to change notification settings - Fork 0
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
Add support for version and deduplication #1
base: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,7 @@ | ||
#!/bin/sh | ||
|
||
if docker build -f docker/Dockerfile -t dd-import:latest .; then | ||
docker run --rm dd-import:latest ./bin/runUnitTests.sh | ||
if docker build -f docker/Dockerfile -t brightercore.azurecr.io/dd-import:latest .; then | ||
docker run --rm brightercore.azurecr.io/dd-import:latest ./bin/runUnitTests.sh | ||
else | ||
exit 1 | ||
fi |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -156,7 +156,7 @@ def test_new_product(self, mockPost, mockEnv): | |
def test_get_engagement_found(self, mockGet, mockEnv): | ||
response = Mock(spec=Response) | ||
response.status_code = 200 | ||
response.text = '{\"count\": 2, \"results\": [{\"id\": 2, \"name\": \"engagement_dev\"}, {\"id\": 3, \"name\": \"engagement\"}]}' | ||
response.text = '{\"count\": 2, \"results\": [{\"id\": 2, \"name\": \"engagement_dev\", \"version\": \"null\"}, {\"id\": 3, \"name\": \"engagement\", \"version\": \"null\"}, {\"id\": 4, \"name\": \"engagement\", \"version\": \"1.0.1\"}]}' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do you have There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. right. Adjusted. In any case, doesn't seem to matter for the test. It passed before and still passes |
||
mockGet.return_value = response | ||
|
||
api = Api() | ||
|
@@ -168,6 +168,27 @@ def test_get_engagement_found(self, mockGet, mockEnv): | |
mockGet.assert_called_once_with(url, headers=self.header, params=payload, verify=True) | ||
response.raise_for_status.assert_called_once() | ||
|
||
@patch('dd_import.environment.Environment') | ||
@patch('requests.get') | ||
@patch.dict('os.environ', {'DD_URL': 'https://example.com', | ||
'DD_API_KEY': 'api_key', | ||
'DD_ENGAGEMENT_NAME': 'engagement', | ||
'DD_ENGAGEMENT_VERSION': '1.0.1'}) | ||
def test_get_engagement_with_version_found(self, mockGet, mockEnv): | ||
response = Mock(spec=Response) | ||
response.status_code = 200 | ||
response.text = '{\"count\": 2, \"results\": [{\"id\": 2, \"name\": \"engagement_dev\", \"version\": \"null\"}, {\"id\": 3, \"name\": \"engagement\", \"version\": \"null\"}, {\"id\": 4, \"name\": \"engagement\", \"version\": \"1.0.1\"}]}' | ||
mockGet.return_value = response | ||
|
||
api = Api() | ||
id = api.get_engagement(self.product_id) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why does this function return the first result? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess you need to select someway. Taking the first is a reasonable option. In the end there is no clear way what to select if there are more than one matches. The only other reasonable way would be to fail the script if more than one entry is found. However this was already like this from the original author, |
||
|
||
self.assertEqual(id, self.engagement_id) | ||
url = 'https://example.com/api/v2/engagements/' | ||
payload = {'name': 'engagement', 'product': self.product_id, 'version': '1.0.1'} | ||
mockGet.assert_called_once_with(url, headers=self.header, params=payload, verify=True) | ||
response.raise_for_status.assert_called_once() | ||
|
||
@patch('dd_import.environment.Environment') | ||
@patch('requests.get') | ||
@patch('dd_import.dd_api.Api.new_engagement') | ||
|
@@ -191,6 +212,50 @@ def test_get_engagement_not_found(self, mockNewEngagement, mockGet, mockEnv): | |
mockNewEngagement.assert_called_once_with(self.product_id) | ||
response.raise_for_status.assert_called_once() | ||
|
||
@patch('dd_import.environment.Environment') | ||
@patch('requests.post') | ||
@patch.dict('os.environ', {'DD_URL': 'https://example.com', | ||
'DD_API_KEY': 'api_key', | ||
'DD_ENGAGEMENT_NAME': 'engagement', | ||
'DD_ENGAGEMENT_VERSION': '1.0.1'}) | ||
def test_new_engagement_with_version(self, mockPost, mockEnv): | ||
response = Mock(spec=Response) | ||
response.status_code = 200 | ||
response.text = '{\"id\": 3}' | ||
mockPost.return_value = response | ||
|
||
api = Api() | ||
id = api.new_engagement(self.product_id) | ||
|
||
self.assertEqual(id, self.engagement_id) | ||
today = datetime.date.today().isoformat() | ||
url = 'https://example.com/api/v2/engagements/' | ||
payload = f'{{"name": "engagement", "product": 2, "target_start": "{today}", "target_end": "2999-12-31", "engagement_type": "CI/CD", "deduplication_on_engagement": false, "status": "In Progress", "version": "1.0.1"}}' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does deduplication_on_engagement influence the version? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No why? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not sure what payload does, but there is here "version": "1.0.1" and line 255, there is no version There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. payload is the expected body of the internal rest request. This tool is "configured" using env vars and creates rest requests accordingly. In line 220 you see that the DD_ENGAGEMENT_VERSION is set. That's why the tool includes version in the rest request. DD_ENGAGEMENT_DEDUPLICATION is not set so the request includes the default value. For Version there is no default. If it isn't defined it will not be in the body. |
||
mockPost.assert_called_once_with(url, headers=self.header, data=payload, verify=True) | ||
response.raise_for_status.assert_called_once() | ||
|
||
@patch('dd_import.environment.Environment') | ||
@patch('requests.post') | ||
@patch.dict('os.environ', {'DD_URL': 'https://example.com', | ||
'DD_API_KEY': 'api_key', | ||
'DD_ENGAGEMENT_NAME': 'engagement', | ||
'DD_ENGAGEMENT_DEDUPLICATION': 'True'}) | ||
def test_new_engagement_with_deduplication(self, mockPost, mockEnv): | ||
response = Mock(spec=Response) | ||
response.status_code = 200 | ||
response.text = '{\"id\": 3}' | ||
mockPost.return_value = response | ||
|
||
api = Api() | ||
id = api.new_engagement(self.product_id) | ||
|
||
self.assertEqual(id, self.engagement_id) | ||
today = datetime.date.today().isoformat() | ||
url = 'https://example.com/api/v2/engagements/' | ||
payload = f'{{"name": "engagement", "product": 2, "target_start": "{today}", "target_end": "2999-12-31", "engagement_type": "CI/CD", "deduplication_on_engagement": true, "status": "In Progress"}}' | ||
mockPost.assert_called_once_with(url, headers=self.header, data=payload, verify=True) | ||
response.raise_for_status.assert_called_once() | ||
|
||
@patch('dd_import.environment.Environment') | ||
@patch('requests.post') | ||
@patch.dict('os.environ', {'DD_URL': 'https://example.com', | ||
|
@@ -208,7 +273,7 @@ def test_new_engagement_without_target(self, mockPost, mockEnv): | |
self.assertEqual(id, self.engagement_id) | ||
today = datetime.date.today().isoformat() | ||
url = 'https://example.com/api/v2/engagements/' | ||
payload = f'{{"name": "engagement", "product": 2, "target_start": "{today}", "target_end": "2999-12-31", "engagement_type": "CI/CD", "status": "In Progress"}}' | ||
payload = f'{{"name": "engagement", "product": 2, "target_start": "{today}", "target_end": "2999-12-31", "engagement_type": "CI/CD", "deduplication_on_engagement": false, "status": "In Progress"}}' | ||
mockPost.assert_called_once_with(url, headers=self.header, data=payload, verify=True) | ||
response.raise_for_status.assert_called_once() | ||
|
||
|
@@ -230,7 +295,7 @@ def test_new_engagement_with_target(self, mockPost, mockEnv): | |
|
||
self.assertEqual(id, self.engagement_id) | ||
url = 'https://example.com/api/v2/engagements/' | ||
payload = '{"name": "engagement", "product": 2, "target_start": "2023-02-01", "target_end": "2023-02-28", "engagement_type": "CI/CD", "status": "In Progress"}' | ||
payload = '{"name": "engagement", "product": 2, "target_start": "2023-02-01", "target_end": "2023-02-28", "engagement_type": "CI/CD", "deduplication_on_engagement": false, "status": "In Progress"}' | ||
mockPost.assert_called_once_with(url, headers=self.header, data=payload, verify=True) | ||
response.raise_for_status.assert_called_once() | ||
|
||
|
@@ -252,7 +317,7 @@ def test_new_engagement_with_source_code_management_uri(self, mockPost, mockEnv) | |
self.assertEqual(id, self.engagement_id) | ||
today = datetime.date.today().isoformat() | ||
url = 'https://example.com/api/v2/engagements/' | ||
payload = f'{{"name": "engagement", "product": 2, "target_start": "{today}", "target_end": "2999-12-31", "engagement_type": "CI/CD", "status": "In Progress", "source_code_management_uri": "https://github.com/MyOrg/MyProject/tree/main"}}' | ||
payload = f'{{"name": "engagement", "product": 2, "target_start": "{today}", "target_end": "2999-12-31", "engagement_type": "CI/CD", "deduplication_on_engagement": false, "status": "In Progress", "source_code_management_uri": "https://github.com/MyOrg/MyProject/tree/main"}}' | ||
mockPost.assert_called_once_with(url, headers=self.header, data=payload, verify=True) | ||
response.raise_for_status.assert_called_once() | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What did you need a deduplication flag?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Different version of a "product" (redact-pipeline, redact-infer) will be tracked as different engaements. I don't want defect dojo to count identical finding across engagements as duplicates. Or otherway around each engagementt representing a differentt version should have it's independent findings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are still only setting a portion of all possible fields.
For reference, these are all the field of an engagement: