Skip to content
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

[dependencies] update go version to 1.22 and update vulnerable dependencies #1034

Merged
merged 1 commit into from
May 17, 2024

Conversation

Shastick
Copy link
Contributor

@Shastick Shastick commented May 16, 2024

This PR updates specific dependencies that have CVE warnings, and updates the Go version from 1.17 (which is not maintained anymore) to 1.22.

Golangci-lint also needed to be updated, and the golint linter removed, as it has no effect anymore.

Notes

I considered only bumping the vulnerable dependencies, but go mod tidy was failing with go 1.17. I believe the test coverage is sufficient for us to do such an update confidently.

A pass of updates on the other dependencies (if any) will happen in an upcoming PR.

This PR also adds a call to go env in the Job Information CI step to help troubleshoot issues with the Go toolchain version being run.

Testing

I ran this with the current master version of the uss_qualifier using all available configurations and did not run into particular issues.(*)

(*) with the exception of a WARNING related to flight planning, which is a common occurrence for me when running all the qualifier configurations locally

@Shastick Shastick force-pushed the go-version-update-and-deps branch 4 times, most recently from e65bb70 to 886a5b4 Compare May 16, 2024 21:15
@Shastick Shastick force-pushed the go-version-update-and-deps branch from 886a5b4 to e954ae4 Compare May 16, 2024 21:22
@BenjaminPelletier BenjaminPelletier linked an issue May 17, 2024 that may be closed by this pull request
@BenjaminPelletier
Copy link
Member

This looks good to me, thanks -- @barroco could you also take a look to make sure I didn't miss anything, especially as may relate to deployment considerations?

(*) with the exception of a WARNING related to flight planning, which is a common occurrence for me when running all the qualifier configurations locally

This is worrying and invalidates running locally as a verification method. Our CI tests essentially never fail in this way, so this seems like something particular to your local environment. If your local environment is not suitable for testing, we would want to fix that in order to enable verification of changes locally. If there is some shortcoming of the project that was causing failures locally but not in the CI, we'd very much want to identify and fix that.

@Shastick
Copy link
Contributor Author

I can reliably run into the problem with the message_signing configuration, which yields the failure below.

Without the message_signing configuration, I can run all configurations (by invoking the qualifier without passing a configuration) and all test suites pass. I checked this twice, starting from a clean local environment each time (with make restart-all): once with the DSS in this PR, once with the currently released DSS.

Should I open an issue on the monitoring repository to address the problem with the message_signing config?

2024-05-17 15:22:47.457 | WARNING  | monitoring.uss_qualifier.suites.suite:_print_failed_check:73 - New failed check:
  details: 'mock_uss indicated flight planning activity PlanningActivityResult.Rejected
    leaving flight plan FlightPlanStatus.NotPlanned rather than the expected (Activity
    PlanningActivityResult.Completed, flight plan FlightPlanStatus.Planned) with no
    notes
  
    Severity Severity.High upgraded to Critical because `stop_fast` flag set true in
    configuration'
  documentation_url: https://github.com/interuss/monitoring/blob/2f5ca5b4965432a53be5426a4b8bb44024b3d5da/monitoring/uss_qualifier/scenarios/flight_planning/plan_flight_intent.md#successful-planning-check
  name: Successful planning
  participants:
  - mock_uss
  query_report_timestamps:
  - '2024-05-17T15:22:47.451551Z'
  requirements:
  - interuss.automated_testing.flight_planning.ExpectedBehavior
  severity: Critical
  summary: Flight planning activity unexpectedly PlanningActivityResult.Rejected leaving
    flight plan FlightPlanStatus.NotPlanned
  timestamp: '2024-05-17T15:22:47.456859Z'

@barroco
Copy link
Collaborator

barroco commented May 17, 2024

Should I open an issue on the monitoring repository to address the problem with the message_signing config?

Yes please open an issue and attach a representative report if you can't fix reliably your environment.

Copy link
Collaborator

@barroco barroco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me as is. I think the next step will be to test it on a full scale deployment, which will be done with the next release.

@barroco barroco merged commit e0bdb36 into interuss:master May 17, 2024
6 checks passed
@Shastick
Copy link
Contributor Author

This looks good to me as is. I think the next step will be to test it on a full scale deployment, which will be done with the next release.

Done -> interuss/monitoring#693

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unmaintained dependency & version
3 participants