Upgrade dependencies 2023-11-27 (#5723) #5735

dsotirho-ucsc · 2023-11-29T19:18:48Z

Connected issues: #5723

Checklist

Author

¹ when the issue title describes a problem, the corresponding PR
title is Fix: followed by the issue title

Author (reindex, API changes)

Added r tag to commit title _{or this PR does not require reindexing}
Added reindex label to PR _{or this PR does not require reindexing}
PR and connected issue are labeled API _{or this PR does not modify a REST API}
Added a (A) tag to commit title for backwards (in)compatible changes _{or this PR does not modify a REST API}
Updated REST API version number in app.py _{or this PR does not modify a REST API}

Author (chains)

This PR is blocked by previous PR in the chain _{or this PR is not chained to another PR}
Added base label to the blocking PR _{or this PR is not chained to another PR}
Added chained label to this PR _{or this PR is not chained to another PR}

Author (upgrading)

Documented upgrading of deployments in UPGRADING.rst _{or this PR does not require upgrading}
Added u tag to commit title _{or this PR does not require upgrading}
Added upgrade label to PR _{or this PR does not require upgrading}

Author (operator tasks)

Added checklist items for additional operator tasks _{or this PR does not require additional tasks}

Author (hotfixes)

Added F tag to main commit title _{or this PR does not include permanent fix for a temporary hotfix}
Reverted the temporary hotfixes for any connected issues _{or the prod branch has no temporary hotfixes for any connected issues}

Author (before every review)

Rebased PR branch on develop, squashed old fixups
Ran make requirements_update _{or this PR does not touch requirements*.txt, common.mk, Makefile and Dockerfile}
Added R tag to commit title _{or this PR does not touch requirements*.txt}
Added reqs label to PR _{or this PR does not touch requirements*.txt}
make integration_test passes in personal deployment _{or this PR does not touch functionality that could break the IT}

Peer reviewer (after requesting changes)

Uncheck the Author (before every review) checklists.

Peer reviewer (after approval)

PR is not a draft
Ticket is in Review requested column
Requested review from primary reviewer
Assigned PR to primary reviewer

Primary reviewer (after requesting changes)

Uncheck the before every review checklists. Update the N reviews label.

Primary reviewer (after approval)

Actually approved the PR
Labeled connected issues as demo or no demo
Commented on connected issues about demo expectations _{or all connected issues are labeled no demo}
Decided if PR can be labeled no sandbox
PR title is appropriate as title of merge commit
N reviews label is accurate
Moved ticket to Approved column
Assigned PR to current operator

Operator (before pushing merge the commit)

Operator (chain shortening)

Changed the target branch of the blocked PR to develop _{or this PR is not labeled base}
Removed the chained label from the blocked PR _{or this PR is not labeled base}
Removed the blocking relationship from the blocked PR _{or this PR is not labeled base}
Removed the base label from this PR _{or this PR is not labeled base}

Operator (after pushing the merge commit)

¹ When pushing the merge commit is skipped due to the PR being
labelled no sandbox, the next build triggered by a PR whose merge commit is
pushed determines this checklist item.

Operator (reindex)

Primary Reviewer

Review exported Inspector findings (Upgrade dependencies 2023-11-27 #5723 (comment))

Operator

Added CL items to prod promotion PR:

Deployed prod.shared

Deployed prod.gitlab

Ran make -C terraform/gitlab/runner (to prod.gitlab)

Unassigned PR

Shorthand for review comments

L line is too long
W line wrapping is wrong
Q bad quotes
F other formatting problem

coveralls · 2023-11-29T20:43:47Z

coverage: 83.071% (+0.005%) from 83.066%
when pulling 1520f6f on issues/dsotirho-ucsc/5723-upgrade-dependencies
into 0bd9865 on develop.

codecov · 2023-11-29T20:44:13Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (0bd9865) 83.04% compared to head (1520f6f) 83.05%.

Additional details and impacted files

@@           Coverage Diff            @@
##           develop    #5735   +/-   ##
========================================
  Coverage    83.04%   83.05%           
========================================
  Files          153      153           
  Lines        19410    19416    +6     
========================================
+ Hits         16119    16125    +6     
  Misses        3291     3291

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

dsotirho-ucsc · 2023-12-01T03:56:21Z

PR was deployed to personal deployment & passed tests including IT.
PR was deployed to GitLab dev (for the GitLab update) and pushed to sandbox. Pipeline 28706 successfully.

achave11-ucsc

Changes look good, nicely done. A few nits.
Also consider doing another pass over your Inspector script, it might benefit from having more type hints and other misc formatting changes.

achave11-ucsc · 2023-12-01T18:25:46Z

.github/ISSUE_TEMPLATE/upgrade.md

-  - [ ] … update to ClamAV image
-  - [ ] … update to GitLab AMI
+  - [ ] … update to [Python](https://hub.docker.com/_/python/tags) (only patch versions) 
+  - [ ] … Updates to [Terraform](https://hub.docker.com/r/hashicorp/terraform/tags) (only patch versions)


U… might be a typo here.

achave11-ucsc · 2023-12-01T18:38:07Z

scripts/inspector-findings.py

+                                 f'Default: {cls.default_severities}\n'
+                                 'Choices: ' + ','.join(cls.all_severities))


Consider adhering to a single format, whether it be f-strings or concatenation, I think it might improve consistency.

achave11-ucsc · 2023-12-01T18:42:48Z

scripts/inspector-findings.py

+                                 f'Default: {cls.default_severities}\n'
+                                 'Choices: ' + ','.join(cls.all_severities))


Also consider a space character after the comma in the join command, to improve readability.

achave11-ucsc · 2023-12-01T18:48:46Z

scripts/inspector-findings.py

+
+    def main(self):
+        log.info('Fetching all findings from AWS Inspector')
+        client = aws.client('inspector2')  # Note inspector2 not inspector


The comment might be redundant, I think it's protocol (has been enforced during review process) to use the most recent client or resource API version available.

achave11-ucsc · 2023-12-01T18:50:48Z

scripts/inspector-findings.py

+        paginator = client.get_paginator('list_findings')
+        for page in paginator.paginate():
+            self.findings.extend(page['findings'])
+        log.info(f'Fetched a total of {len(self.findings)} findings from AWS Inspector')


Reference the fourth bulleted item in https://github.com/DataBiosphere/azul/blob/develop/CONTRIBUTING.rst#logging.

achave11-ucsc · 2023-12-01T18:56:52Z

scripts/inspector-findings.py

+        log.info(f'Found {finding_count} vulnerabilities with a severity '
+                 f'{self.severities!r}')
+        log.info(f'Grouped findings by {len(findings)} unique vulnerabilities')


achave11-ucsc · 2023-12-01T18:57:13Z

scripts/inspector-findings.py

+            file_data.append(row)
+
+        output_file_name = f'inspector-findings_{self.date}.csv'
+        log.info(f'Writing file {output_file_name!r}')


achave11-ucsc · 2023-12-01T19:01:33Z

scripts/inspector-findings.py

+            if severity not in self.severities:
+                continue


This looks to me like what the first bulleted item in https://github.com/DataBiosphere/azul/blob/develop/CONTRIBUTING.rst#control-flow describes.

achave11-ucsc · 2023-12-01T19:02:57Z

scripts/inspector-findings.py

+            assert summary['instances'] or summary['images'], summary
+            finding_count += 1
+            findings[vulnerability].append(summary)
+        log.info(f'Found {finding_count} vulnerabilities with a severity '


Would len(findings) not achieve the same here?
The counter might be unnecessary.

finding_count is the total number of findings (that matched the specified severity), while len(findings) gives the number of unique vulnerabilities (e.g. CVE-1234), which can have one or more finding associated with it. Not entirely necessary as log output however, I'll clean this up.

achave11-ucsc · 2023-12-01T19:10:10Z

scripts/inspector-findings.py

+        self.write_compact_csv()
+        log.info('Done.')
+
+    def parse_findings_for_csv(self) -> tuple[dict, set, set]:


The definition of this method seems a bit complicated. Consider doing another pass over this method to see if it might do with a bit of de-duplication or simplification.

Additionally, a bit of preprocessing might help cleanup and address what looks like a bail-out.

dsotirho-ucsc · 2023-12-02T01:34:05Z

Note: I amended the Update GitLab… commit (instead of adding a fixup) to change the new version to 6.16.1 (instead of 6.16.0).

PR was redeployed to dev.gitlab to test GitLab 6.16.1, and the PR passed on sandbox (Pipeline 28731)

achave11-ucsc

🦭✅

hannes-ucsc · 2023-12-05T01:02:20Z

When I rebased, some of the commits were squashed out. I also ran make requirements_update again.

This reverts commit 8f0fcf4.

github-actions bot added the orange [process] Done by the Azul team label Nov 29, 2023

dsotirho-ucsc force-pushed the issues/dsotirho-ucsc/5723-upgrade-dependencies branch from 9102660 to 929a5e3 Compare November 29, 2023 20:26

dsotirho-ucsc force-pushed the issues/dsotirho-ucsc/5723-upgrade-dependencies branch 4 times, most recently from 3f7bcc5 to c597169 Compare December 1, 2023 01:19

dsotirho-ucsc added the upgrade [process] PR includes commit requiring manual upgrade label Dec 1, 2023

dsotirho-ucsc force-pushed the issues/dsotirho-ucsc/5723-upgrade-dependencies branch from c597169 to 7abca52 Compare December 1, 2023 01:32

dsotirho-ucsc added the reqs [process] PR includes commit requiring ``make requirements`` label Dec 1, 2023

dsotirho-ucsc requested a review from achave11-ucsc December 1, 2023 03:56

dsotirho-ucsc assigned achave11-ucsc Dec 1, 2023

achave11-ucsc requested changes Dec 1, 2023

View reviewed changes

achave11-ucsc removed their assignment Dec 1, 2023

dsotirho-ucsc force-pushed the issues/dsotirho-ucsc/5723-upgrade-dependencies branch 2 times, most recently from 8b0b2e2 to 3cfe124 Compare December 2, 2023 00:19

dsotirho-ucsc requested a review from achave11-ucsc December 2, 2023 01:34

dsotirho-ucsc assigned achave11-ucsc Dec 2, 2023

achave11-ucsc previously approved these changes Dec 2, 2023

View reviewed changes

achave11-ucsc marked this pull request as ready for review December 2, 2023 04:08

achave11-ucsc requested a review from hannes-ucsc as a code owner December 2, 2023 04:08

achave11-ucsc assigned hannes-ucsc and unassigned achave11-ucsc Dec 2, 2023

hannes-ucsc dismissed achave11-ucsc’s stale review via ba0d08a December 5, 2023 00:59

hannes-ucsc force-pushed the issues/dsotirho-ucsc/5723-upgrade-dependencies branch from 3cfe124 to ba0d08a Compare December 5, 2023 00:59

hannes-ucsc previously approved these changes Dec 5, 2023

View reviewed changes

hannes-ucsc added the 0 reviews [process] Lead didn't request any changes label Dec 5, 2023

hannes-ucsc removed their assignment Dec 5, 2023

dsotirho-ucsc added 4 commits December 5, 2023 16:07

Revert "Inline azul_docker_pycharm_version"

b638b1a

This reverts commit 8f0fcf4.

Add docker_elasticsearch_version property

0ce0855

Update Upgrade dependencies issue template

800c5b2

Add script to fetch inspector findings (#5723)

eeb3e22

dsotirho-ucsc dismissed hannes-ucsc’s stale review via da6ffd6 December 6, 2023 00:25

dsotirho-ucsc force-pushed the issues/dsotirho-ucsc/5723-upgrade-dependencies branch from ba0d08a to da6ffd6 Compare December 6, 2023 00:25

dsotirho-ucsc and others added 10 commits December 5, 2023 16:51

[u] Update Python to 3.11.6 (#5723)

aeb0103

[u] Update Terraform to 1.3.10 (#5723)

4e56804

[u] Update PyCharm to 2023.5.6-6 (#5723)

ac77930

[u] Update Elasticsearch to 7.17.15-6 (#5723)

b1c15ed

[u] Update Docker to 24.0.7 (#5723)

d13b016

[u] Update GitLab to 16.6.1 and runner to 16.6.1 (#5723)

16a1849

[u] Update ClamAV to 1.2.1-17 (#5723)

e8001aa

[u] Update GitLab AMI to v2.0.0.26 (#5723)

a0e3f08

[R] Ran make requirements_update

9da90cd

[R] Ran make requirements_update again

1520f6f

dsotirho-ucsc force-pushed the issues/dsotirho-ucsc/5723-upgrade-dependencies branch from da6ffd6 to 1520f6f Compare December 6, 2023 00:53

dsotirho-ucsc added the sandbox [process] Resolution is being verified in sandbox deployment label Dec 6, 2023

nadove-ucsc mentioned this pull request Dec 6, 2023

Suppress Inspector findings for images not used within boundary (#5738) #5758

Merged

dsotirho-ucsc requested a review from hannes-ucsc December 6, 2023 21:35

hannes-ucsc approved these changes Dec 6, 2023

View reviewed changes

dsotirho-ucsc merged commit 33ec475 into develop Dec 6, 2023
8 checks passed

dsotirho-ucsc deleted the issues/dsotirho-ucsc/5723-upgrade-dependencies branch December 7, 2023 00:39

achave11-ucsc assigned achave11-ucsc and hannes-ucsc and unassigned dsotirho-ucsc and achave11-ucsc Dec 11, 2023

hannes-ucsc removed their assignment Dec 19, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade dependencies 2023-11-27 (#5723) #5735

Upgrade dependencies 2023-11-27 (#5723) #5735

dsotirho-ucsc commented Nov 29, 2023 •

edited by hannes-ucsc

Loading

coveralls commented Nov 29, 2023 •

edited

Loading

codecov bot commented Nov 29, 2023 •

edited

Loading

dsotirho-ucsc commented Dec 1, 2023

achave11-ucsc left a comment

achave11-ucsc Dec 1, 2023

achave11-ucsc Dec 1, 2023

achave11-ucsc Dec 1, 2023

achave11-ucsc Dec 1, 2023

achave11-ucsc Dec 1, 2023

achave11-ucsc Dec 1, 2023

achave11-ucsc Dec 1, 2023

achave11-ucsc Dec 1, 2023

achave11-ucsc Dec 1, 2023

dsotirho-ucsc Dec 1, 2023 •

edited

Loading

achave11-ucsc Dec 1, 2023

achave11-ucsc Dec 1, 2023

dsotirho-ucsc commented Dec 2, 2023 •

edited

Loading

achave11-ucsc left a comment

hannes-ucsc commented Dec 5, 2023

		f'Default: {cls.default_severities}\n'
		'Choices: ' + ','.join(cls.all_severities))

Upgrade dependencies 2023-11-27 (#5723) #5735

Upgrade dependencies 2023-11-27 (#5723) #5735

Conversation

dsotirho-ucsc commented Nov 29, 2023 • edited by hannes-ucsc Loading

Checklist

Author

Author (reindex, API changes)

Author (chains)

Author (upgrading)

Author (operator tasks)

Author (hotfixes)

Author (before every review)

Peer reviewer (after requesting changes)

Peer reviewer (after approval)

Primary reviewer (after requesting changes)

Primary reviewer (after approval)

Operator (before pushing merge the commit)

Operator (chain shortening)

Operator (after pushing the merge commit)

Operator (reindex)

Primary Reviewer

Operator

Shorthand for review comments

coveralls commented Nov 29, 2023 • edited Loading

codecov bot commented Nov 29, 2023 • edited Loading

Codecov Report

dsotirho-ucsc commented Dec 1, 2023

achave11-ucsc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dsotirho-ucsc Dec 1, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dsotirho-ucsc commented Dec 2, 2023 • edited Loading

achave11-ucsc left a comment

Choose a reason for hiding this comment

hannes-ucsc commented Dec 5, 2023

dsotirho-ucsc commented Nov 29, 2023 •

edited by hannes-ucsc

Loading

coveralls commented Nov 29, 2023 •

edited

Loading

codecov bot commented Nov 29, 2023 •

edited

Loading

dsotirho-ucsc Dec 1, 2023 •

edited

Loading

dsotirho-ucsc commented Dec 2, 2023 •

edited

Loading