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

Update cloudgov.py #2730

Merged
merged 9 commits into from
Nov 2, 2023
Merged

Conversation

andrew-jameson
Copy link
Collaborator

@andrew-jameson andrew-jameson commented Oct 16, 2023

Summary of Changes

This PR is a hotfix in coordination with manual changes performed in the staging space for our crashed develop app. The main jist of this changeset is correcting the variable name to CGAPPNAME_BACKEND in deploy-backend.sh as this was incorrectly attempting to bind to incorrect services. Additionally, the incorrect JWT_KEY variable was being assigned to develop since raft's CircleCI would automatically attempt to utilize the dev space's Cloud.gov connector and key.

How to Test

  1. CircleCI will need to be updated with environment variables prefixed with "STAGING_" to be ingested and set in the cloud.gov app.
  2. Once merged, this should automatically kick off our deployment to develop and we should see a successful deployment workflow. Same process should be followed for HHS:main.

Deliverables

More details on how deliverables herein are assessed included here.

Deliverable 1: Accepted Features

Checklist of ACs:

  • [insert ACs here]
  • lfrohlich and/or adpennington confirmed that ACs are met.

Deliverable 2: Tested Code

  • Are all areas of code introduced in this PR meaningfully tested?
    • If this PR introduces backend code changes, are they meaningfully tested?
    • If this PR introduces frontend code changes, are they meaningfully tested?
  • Are code coverage minimums met?
    • Frontend coverage: [insert coverage %] (see CodeCov Report comment in PR)
    • Backend coverage: [insert coverage %] (see CodeCov Report comment in PR)

Deliverable 3: Properly Styled Code

  • Are backend code style checks passing on CircleCI?
  • Are frontend code style checks passing on CircleCI?
  • Are code maintainability principles being followed?

Deliverable 4: Accessible

  • Does this PR complete the epic?
  • Are links included to any other gov-approved PRs associated with epic?
  • Does PR include documentation for Raft's a11y review?
  • Did automated and manual testing with iamjolly and ttran-hub using Accessibility Insights reveal any errors introduced in this PR?

Deliverable 5: Deployed

  • Was the code successfully deployed via automated CircleCI process to development on Cloud.gov?

Deliverable 6: Documented

  • Does this PR provide background for why coding decisions were made?
  • If this PR introduces backend code, is that code easy to understand and sufficiently documented, both inline and overall?
  • If this PR introduces frontend code, is that code easy to understand and sufficiently documented, both inline and overall?
  • If this PR introduces dependencies, are their licenses documented?
  • Can reviewer explain and take ownership of these elements presented in this code review?

Deliverable 7: Secure

  • Does the OWASP Scan pass on CircleCI?
  • Do manual code review and manual testing detect any new security issues?
  • If new issues detected, is investigation and/or remediation plan documented?

Deliverable 8: User Research

Research product(s) clearly articulate(s):

  • the purpose of the research
  • methods used to conduct the research
  • who participated in the research
  • what was tested and how
  • impact of research on TDP
  • (if applicable) final design mockups produced for TDP development

@raftmsohani
Copy link

Had to bind develop services:

cf bind-service tdp-backend-develop tdp-db-develop
cf bind-service tdp-backend-develop tdp-staticfiles-develop
cf bind-service tdp-backend-develop tdp-datafiles-develop 

and then cf restage tdp-backend-develop

@raftmsohani
Copy link

raftmsohani commented Oct 16, 2023

Had to bind develop services:

cf bind-service tdp-backend-develop tdp-db-develop
cf bind-service tdp-backend-develop tdp-staticfiles-develop
cf bind-service tdp-backend-develop tdp-datafiles-develop 

and then cf restage tdp-backend-develop

The get_cloudgov_service_creds_by_instance_name logic in settings file looks for develop services wince the app name is service, since these were not binded, the VCAP service name was empty causing the settings file to fail

@andrew-jameson andrew-jameson self-assigned this Oct 18, 2023
@andrew-jameson andrew-jameson marked this pull request as ready for review October 18, 2023 14:13
@@ -35,6 +35,7 @@
branches:
only:
- develop
- debug/develop-deployment-failures

Choose a reason for hiding this comment

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

remove before merge

@codecov
Copy link

codecov bot commented Oct 20, 2023

Codecov Report

Merging #2730 (437f893) into develop (8ec31f9) will decrease coverage by 0.58%.
Report is 48 commits behind head on develop.
The diff coverage is 91.53%.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #2730      +/-   ##
===========================================
- Coverage    92.99%   92.41%   -0.58%     
===========================================
  Files          219      239      +20     
  Lines         4482     5340     +858     
  Branches       385      473      +88     
===========================================
+ Hits          4168     4935     +767     
- Misses         242      312      +70     
- Partials        72       93      +21     
Flag Coverage Δ
dev-backend 92.29% <92.06%> (-0.55%) ⬇️
dev-frontend 92.95% <78.26%> (-0.65%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
tdrs-backend/tdpservice/core/logger.py 100.00% <100.00%> (ø)
tdrs-backend/tdpservice/data_files/serializers.py 100.00% <100.00%> (ø)
...rs-backend/tdpservice/data_files/test/factories.py 100.00% <100.00%> (ø)
tdrs-backend/tdpservice/data_files/validators.py 96.15% <ø> (ø)
tdrs-backend/tdpservice/data_files/views.py 90.65% <100.00%> (+0.08%) ⬆️
tdrs-backend/tdpservice/email/email.py 76.78% <100.00%> (-1.55%) ⬇️
...vice/email/helpers/account_deactivation_warning.py 100.00% <100.00%> (ø)
...backend/tdpservice/email/helpers/account_status.py 84.84% <100.00%> (ø)
tdrs-backend/tdpservice/email/helpers/data_file.py 100.00% <100.00%> (ø)
tdrs-backend/tdpservice/parsers/admin.py 100.00% <100.00%> (ø)
... and 53 more

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 932a41e...437f893. Read the comment docs.

Copy link

@George-Hudson George-Hudson left a comment

Choose a reason for hiding this comment

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

Once the branches tracked are reverted back to develop only, I think this is good to go

@@ -62,6 +64,8 @@ set_cf_envs()
cf_cmd="cf unset-env $CGAPPNAME_BACKEND $var_name ${!var_name}"
$cf_cmd
continue
elif [[ ("$var_name" =~ "STAGING_*") && ("$CF_SPACE" = "tanf-staging") ]]; then
Copy link

Choose a reason for hiding this comment

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

Should this be ("$CF_SPACE" == "tanf-staging")?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Initially thought the same thing but saw the single = used in other comparisons so went with what was used elsewhere.

Copy link

@raftmsohani raftmsohani left a comment

Choose a reason for hiding this comment

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

lgtml

Copy link
Collaborator

@ADPennington ADPennington left a comment

Choose a reason for hiding this comment

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

@andrew-jameson this is ready to merge. thanks for the summary. added a comment in-line re: jwt_key.

Update: just saw the note about prefix. good to go!

@andrew-jameson andrew-jameson merged commit a886f70 into develop Nov 2, 2023
10 checks passed
@andrew-jameson andrew-jameson deleted the debug/develop-deployment-failures branch November 2, 2023 13:44
@andrew-jameson andrew-jameson restored the debug/develop-deployment-failures branch November 3, 2023 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants