search

department-of-veterans-affairs / notification-api

Notification API
MIT License
16 stars 9 forks source link

Regular update for Dependencies #1472

Closed mjones-oddball closed 1 year ago

mjones-oddball commented 1 year ago

User Story - Business Need

We wish to keep dependencies up to date so we do not need such massive overhauls of our system. This will be a recurring ticket that will be done every sprint. We will update all dependencies we are able to. Any conflicts will get a ticket. This is intended to be a day of work at most because this is intended to update with only non-breaking changes.

User Story

As VA Notify I want to keep our service up to date So that we are secure and as free of bugs as possible

Additional Info and Resources

Engineering Checklist

Acceptance Criteria

Repo dependencies are updated and we have no broken functionality. Issues opened by Dependabot are resolved. Tickets created for any updates we could, or should, not do.

QA Considerations

cris-oddball commented 1 year ago

@EvanParish noting that the click Dependbot issue could not be closed out the last time because requirements-cli.txt was not updated with click. Can you be sure to take a look at that file, please?

EvanParish commented 1 year ago

When attempting to test the dependency updates the regression suit was run against the deployed code and something in this branch caused it to run for 30+ min before we stopped it. Going back to master we saw it take the usual < 5 min amount of time. I'm working on figuring out what might be causing the issue.

EvanParish commented 1 year ago

I rolled back all of the patch updates and deployed with just minor version updates. This seems to have solved the issue, but I'm not sure which package caused the delayed sending in the first place. I suspect that future updates probably won't have that issue, so it's most likely just a waste of time finding the specific one.

cris-oddball commented 1 year ago

Sending approved and merged PR to Perf on tag v1.6.13-1472-dependencies. Doesn't look like any Twistlock issues can be cleared here.

cris-oddball commented 1 year ago

Click dependabot issue cleared on this one.

cris-oddball commented 1 year ago

A number of failed tests on the first run

 FAILED tests/test_communication_item.py::test_get_communication_item_invalid_communication_item_404
FAILED tests/test_service.py::test_get_all_services - AssertionError: Invalid...
FAILED tests/test_service_api_key.py::test_get_an_api_key - AssertionError: I...
FAILED tests/test_service_template.py::test_get_all_templates - AssertionErro...
FAILED tests/test_v2_notifications_email.py::test_send_email[Email: Basic Formatting w/ EDIPI and Optional Properties]
FAILED tests/test_v3_notifications_email.py::test_send_v3_email[Email: Basic Formatting]
FAILED tests/test_v3_notifications_email.py::test_send_v3_email_invalid_payload_400[400 EMAIL: Missing required property 'template_id']
FAILED tests/test_v3_notifications_email.py::test_send_v3_email_invalid_payload_400[400 EMAIL: Invalid type 'client_reference' not a string]
FAILED tests/test_v3_notifications_sms.py::test_send_v3_sms[SMS: Short w/ sms and EDIPI recipient ID]
FAILED tests/test_v3_notifications_sms.py::test_send_v3_sms_invalid_auth_401_403[SMS 403: invalid bearer token]

Re-running tests, might be related to the same issue as when the regression is run immediately after the deploy to Staging. The tag Push to Perf also runs the regression immediately.

cris-oddball commented 1 year ago

Now even more test failures on run 2

FAILED tests/test_communication_item.py::test_create_communication_item[Create Communication Item w Default Send Indicator as true]
FAILED tests/test_communication_item.py::test_create_communication_item[Create Communication Item w Default Send Indicator as false]
FAILED tests/test_communication_item.py::test_get_by_communication_item_id - ...
FAILED tests/test_communication_item.py::test_update_communication_item_negative_tests_400[Payload Name non-unique]
FAILED tests/test_communication_item.py::test_get_communication_item_invalid_bearer_token_401[GET All]
FAILED tests/test_communication_item.py::test_create_communication_item_negative_tests_400[Payload VAProfile Item ID non-unique]
FAILED tests/test_service_sms_sender.py::test_get_all_sms_senders - Assertion...
FAILED tests/test_v2_notifications_email.py::test_get_email_notification_statuses
FAILED tests/test_v2_notifications_sms.py::test_get_sms_notification_statuses
FAILED tests/test_v2_notifications_sms.py::test_send_sms_incorrect_format_phone_number_400
FAILED tests/test_v3_notifications_email.py::test_send_v3_email[Email: Basic Formatting w/ BIRLSID]
FAILED tests/test_v3_notifications_email.py::test_send_v3_email_invalid_auth_401_403[SMS 403: invalid bearer token]
FAILED tests/test_v3_notifications_email.py::test_send_v3_email_invalid_payload_400[400 EMAIL: Invalid type 'template_id' not a UUID]
FAILED tests/test_v3_notifications_email.py::test_send_v3_email_invalid_payload_400[400 EMAIL: Invalid type 'email_address' not a string]
FAILED tests/test_v3_notifications_email.py::test_send_v3_email_invalid_payload_400[400 EMAIL: Invalid type 'client_reference' not a string]
FAILED tests/test_v3_notifications_email.py::test_send_v3_email_invalid_payload_400[400 EMAIL: Invalid type 'billing_code' not a string]
FAILED tests/test_v3_notifications_sms.py::test_send_v3_sms[SMS: Short w/ alternate Pinpoint SMS Sender ID]
FAILED tests/test_v3_notifications_sms.py::test_send_v3_sms_invalid_payload_400[400 SMS: Invalid format 'phone_number']
============ 18 failed, 172 passed, 3 xfailed in 264.17s (0:04:24) =============

I need to investigate this prior to running any more regressions.

cris-oddball commented 1 year ago

It is read timeouts all the way down.

cris-oddball commented 1 year ago

Running the regression from the QA repo against the private endpoints now.

cris-oddball commented 1 year ago

Private regression failed due to a communication item not being properly deleted on the previous test run. Re-running.

cris-oddball commented 1 year ago

Regression passes in Perf ================== 190 passed, 3 xfailed in 172.58s (0:02:52) ==================

Holding ticket for the remaining dependabot upgrades.

cris-oddball commented 1 year ago

Dependbot cryptography issue merged and sent to Perf on v1.6.13-1472-cryptography. Regression fails against public endpoints with read timeouts, running regression from QA repo against private endpoints.

cris-oddball commented 1 year ago

Regression passes ================== 190 passed, 3 xfailed in 160.74s (0:02:40) ==================

The next dependabot issues are all for actions.

cris-oddball commented 1 year ago

For the dependabot docker/build-push-action upgrade, am sending master up to Perf using the Deploy to Env action.

Will wait for the cron to run tonight on Update cached Docker image.

EvanParish commented 1 year ago

Created ticket up update phonenumbers package in utils repo: https://app.zenhub.com/workspaces/va-notify-620d21369d810a00146ed9c8/issues/gh/department-of-veterans-affairs/notification-utils/143

cris-oddball commented 1 year ago

Have removed the run-qa-suite from the 3 workflows and merged to master. Using Deploy to Env to send up docker/build-push-action (which will actually test that action).

cris-oddball commented 1 year ago

Build/Push validated as working correctly.

The next PR combines and touches a number of workflows, including Build, User Flows, Locust, DB Downgrade, Deploy Lambda, Deployment, Regression (not going to bother testing), Tests, Twistlock, Update Cached Image (will wait for cron), Datadog Image Update. Will come up with a test plan after it is merged.

cris-oddball commented 1 year ago

Validation checklist for combined PR for workflow updates:

cris-oddball commented 1 year ago

Who knows if the locust email load test ever worked. Has not been run before, I ran it against perf and dev and kept getting 

Waiter CommandExecuted failed: Max attempts exceeded. Previously accepted state: For expression "Status" we matched expected path: "InProgress"

The locust instance is up and running, reporting healthy status checks.

cris-oddball commented 1 year ago

Update cached docker image works as expected.

Closing as passed!

EvanParish commented 1 year ago

I felt that I should leave my notes here about the rest of the dependencies.

Dependabot PR Test failures: