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

API-26148-526-v2-pdf-gen-setup #12663

Merged
merged 0 commits into from
Jun 5, 2023
Merged

Conversation

stiehlrod
Copy link
Contributor

@stiehlrod stiehlrod commented May 15, 2023

Summary

Sets up connection to 526 PDF Generator service
DVP QA → vets-api Dev
DVP Staging → vets-api Staging
DVP Lab → vets-api Sandbox

Related issue(s)

Testing done

What areas of the site does it impact?

(Describe what parts of the site are impacted andifcode touched other areas)

Acceptance criteria

  • I fixed|updated|added unit tests and integration tests for each feature (if applicable).
  • No error nor warning in the console.
  • Events are being sent to the appropriate logging solution
  • Documentation has been updated (link to documentation)
  • No sensitive information (i.e. PII/credentials/internal URLs/etc.) is captured in logging, hardcoded, or specs
  • Feature/bug has a monitor built into Datadog or Grafana (if applicable)
  • If app impacted requires authentication, did you login to a local build and verify all authenticated routes work as expected
  • I added a screenshot of the developed feature

@stiehlrod stiehlrod added Lighthouse lighthouse claimsApi modules/claims_api labels May 15, 2023
@stiehlrod stiehlrod changed the title Adds to setting files the url for the pdf gen API-26148-526-v2-pdf-gen-setup May 15, 2023
@stiehlrod stiehlrod closed this May 15, 2023
@va-vfs-bot va-vfs-bot temporarily deployed to API-26148-526-v2-pdf-gen-setup/main/main May 15, 2023 20:20 Inactive
@stiehlrod stiehlrod reopened this May 15, 2023
@va-vsp-bot va-vsp-bot requested a deployment to API-26148-526-v2-pdf-gen-setup/main/main May 15, 2023 22:18 In progress
@va-vsp-bot va-vsp-bot requested a deployment to API-26148-526-v2-pdf-gen-setup/main/main May 15, 2023 22:20 In progress
@va-vsp-bot va-vsp-bot requested a deployment to API-26148-526-v2-pdf-gen-setup/main/main May 15, 2023 22:21 In progress
@va-vfs-bot va-vfs-bot temporarily deployed to API-26148-526-v2-pdf-gen-setup/main/main May 15, 2023 22:22 Inactive
@stiehlrod stiehlrod marked this pull request as ready for review May 15, 2023 22:37
@stiehlrod stiehlrod requested review from a team as code owners May 15, 2023 22:37
@va-vsp-bot va-vsp-bot requested a deployment to API-26148-526-v2-pdf-gen-setup/main/main May 16, 2023 13:18 In progress
@va-vfs-bot va-vfs-bot temporarily deployed to API-26148-526-v2-pdf-gen-setup/main/main May 16, 2023 13:19 Inactive
@va-vsp-bot va-vsp-bot requested a deployment to API-26148-526-v2-pdf-gen-setup/main/main May 17, 2023 17:55 In progress
@va-vfs-bot va-vfs-bot temporarily deployed to API-26148-526-v2-pdf-gen-setup/main/main May 17, 2023 18:10 Inactive
@va-vsp-bot va-vsp-bot requested a deployment to API-26148-526-v2-pdf-gen-setup/main/main May 17, 2023 22:25 In progress
@va-vfs-bot va-vfs-bot temporarily deployed to API-26148-526-v2-pdf-gen-setup/main/main May 17, 2023 22:26 Inactive
Copy link
Contributor

@mchristiansonVA mchristiansonVA left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -7,7 +7,7 @@ module Dvp
# points to the Digital Veterans Platform
class Configuration < EVSS::DisabilityCompensationForm::Configuration
def base_path
"#{Settings.evss.dvp.url}/#{Settings.evss.dvp.service_name}/rest/form526/v2"
"#{Settings.dvp.url}/#{Settings.evss.service_name}/rest/form526/v2"
Copy link
Contributor

Choose a reason for hiding this comment

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

I see what Settings.evss.service_name connects to in settings.yaml:

evss:
  prefill: true
  url: https://csraciapp6.evss.srarad.com
  service_name: "wss-form526-services-web"     <--------
  alternate_service_name: "wss-form526-services-web-v2"
  cert_path: ~
  key_path: ~
  root_cert_path: ~
  versions:
    claims: 3.6
    common: 11.6
    documents: 3.7

But I don't see what Settings.dvp.url connects to (nor do I see what Settings.evss.dvp.url used to connect to 🤔)

Copy link
Contributor

Choose a reason for hiding this comment

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

oh, I see. It looks like you were trying to reference the files in the devops repo: https://github.com/department-of-veterans-affairs/devops/pull/13019/files. Settings. is looking in settings.yaml in the vets-api repo, not the devops repo.

Copy link
Contributor

Choose a reason for hiding this comment

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

but the environment specific .local.yml files get added during the builds, correct? For example, a Firefly engineer added the original dvp url, shown here... and the dev console is showing such. Or is this removed from the build now? I remember a slack message recently saying that maybe only prod is getting the settings file still?

irb(main):001:0> Settings.evss.dvp.url
=> "https://blue.qa.lighthouse.va.gov"

and if that's the case, this PR would make the needed changes to dev, correct? I'm not familiar with the manifest repo, so correct me if I'm wrong.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i was replacing a reference to a setting that we changed in vets-api settings.yml

Copy link
Contributor

Choose a reason for hiding this comment

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

but the environment specific .local.yml files get added during the builds, correct?

no, not anymore. Everything in this section of values.yaml gets mounted to /app/config/settings.local.yml in the pod. So the reason you're able to see Settings.evss.dvp.url (in a dev pod) is because of the settings defined in the dev values.yaml file. Here's a portion of the settings.local.yml file in a vets-api-prod pod:

nonroot@vets-api-web-5b568b877-4m45b:~$ tail -20 /app/config/settings.local.yml
    aws_access_key_id: <%= ENV['DHP_AWS_ACCESS_KEY_ID'] %>
    aws_secret_access_key: <%= ENV['DHP_AWS_SECRET_ACCESS_KEY'] %>
    region: <%= ENV['DHP_AWS_REGION'] %>
    bucket: <%= ENV['DHP_TOKEN_STORAGE_BUCKET'] %>

dgi:
  jwt:
    public_key_path: /srv/vets-api/secret/production/dgi-prod.crt
    private_key_path: /srv/vets-api/secret/dgi-prod.key
  vets:
    url: <%= ENV['DGI_VETS_URL'] %>

lgy:
  base_url: <%= ENV['LGY_BASE_URL'] %>
  app_id: VAGOVSERVICE
  api_key: <%= ENV['LGY_API_KEY'] %>
  mock_coe: false

dogstatsd:
  enabled: true
nonroot@vets-api-web-5b568b877-4m45b:~$

Copy link
Contributor

Choose a reason for hiding this comment

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

Does that make sense?
With EKS, the settings get loaded in from the settings-configmap in values.yaml in the manifest repo. settings.yml in vets-api has default values but then the values per environment will get loaded in from the configmap. It's best practice to keep vets-api settings.yml and the configmaps inline even if its blank. So settings.yml needs to have something like this

evss:
  dvp:
    url: ~

@stiehlrod stiehlrod requested review from rmtolmach and FonzMP May 31, 2023 14:41
@va-vsp-bot va-vsp-bot requested a deployment to API-26148-526-v2-pdf-gen-setup/main/main May 31, 2023 14:41 In progress
@va-vsp-bot va-vsp-bot requested a deployment to API-26148-526-v2-pdf-gen-setup/main/main May 31, 2023 14:55 In progress
@va-vfs-bot va-vfs-bot temporarily deployed to API-26148-526-v2-pdf-gen-setup/main/main May 31, 2023 14:57 Inactive
@va-vsp-bot va-vsp-bot requested a deployment to API-26148-526-v2-pdf-gen-setup/main/main June 2, 2023 19:17 In progress
@stiehlrod stiehlrod requested a review from ryan-mcneil June 2, 2023 19:18
@va-vfs-bot va-vfs-bot temporarily deployed to API-26148-526-v2-pdf-gen-setup/main/main June 2, 2023 19:18 Inactive
Copy link
Contributor

@ryan-mcneil ryan-mcneil left a comment

Choose a reason for hiding this comment

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

Looks good now!

@va-vsp-bot va-vsp-bot requested a deployment to API-26148-526-v2-pdf-gen-setup/main/main June 5, 2023 13:05 In progress
@stiehlrod stiehlrod merged commit 3f76843 into master Jun 5, 2023
@stiehlrod stiehlrod deleted the API-26148-526-v2-pdf-gen-setup branch June 5, 2023 13:13
ryan-mcneil pushed a commit that referenced this pull request Dec 11, 2023
* Adds to setting files the url for the pdf gen

* REmoves unneeded changes, and changes the settings path to dvp

* Removes line changes

* Removes line changes

* Changes location of evss service name

* Changes location of variables after discussion

* Adds dvp url to settings file

* Corrects values to match config map in the vsp-infra-application-manifests repo.

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

Successfully merging this pull request may close these issues.

7 participants