Converting Rails config to railsconfig

[knkski]

Fixes https://github.com/department-of-veterans-affairs/vets.gov-team/issues/1299

The major advantages of railsconfig over the other options are that it supports YAML syntax unlike dotenv, and it supports local overrides unlike Figaro. It is also is easier to configure than Figaro, since the YAML can be grouped into blocks and doesn't have to look like ES_CLIENT_KEY_PATH: foo.

[knkski]

Went with a fake url since the VA is touchy about real endpoints being in a public repo. This can be overridden for dev by putting something like this into config/settings.local.yml:

hca:
  endpoint: https://some-real-endpoint.va.gov

There was also some weird name conflict with the EVSS stuff, hence https://test-foo.vets.gov instead of just https://test.vets.gov

[jkassemi]

The override options may provide some flexibility with deployment configuration, which could prove useful. So 👍 there.

I prefer when platform optimization settings are defined with a RAILS_ENV, and application configuration is not dependent on the RAILS_ENV more in line with https://12factor.net/config. If we need to start using varying RAILS_ENVs then we'll need some changes to the devops deployments, which all run in production, along with some thought to how this affects the review instances.

Depending on how other folks on the application development teams feel, it'd likely be worth applying globally vs maintaining multiple configuration gems (figaro + railsconfig).

[knkski]

We can use this as we use Figaro now, with theproduction RAILS_ENV being used for everything but local dev. We would just plop down a config/settings.local.yml with the appropriate values when deploying. Basically, I think the major difference in this approach is that we don't use env vars by default (which is something that the 12 factor approach gets wrong, imo), we use unversioned settings files.

And yeah, whatever we choose for this I'd like to apply globally for all app configuration.

[leannammiller]

@knkski can you pls provide an update here?

[knkski]

@leannammiller: Sure. This is mostly done, just needs some testing. I'm working on getting the devops side of things ready to go: https://github.com/department-of-veterans-affairs/devops/pull/1520. I expect to have that ready to push to dev by tomorrow.

[aub]

I like this approach a lot better than using environment variables, although I'm concerned about the size of the change here. Could we have changed one piece of the app at a time so we didn't have to make such a big single PR? Also, can we add some content to the PR instructing people about how to setup the configuration locally?

[aub]

Why do you need this?

[knkski]

Apparently I don't. I was running into an issue with a failing unit test, but I think it was actually unrelated to this.

[aub]

I'm concerned about these things where if the test raises an exception then it could cause the value to not be set back to false. Presumably that's what the ClimateControl thing handles by doing it in a block.

[knkski]

Sure, I was wondering if there was a nice way to do that in Ruby other than just something like this:

begin
  Settings.foo = true
  expect(something).to eq(true)
ensure
  Settings.foo = false
end

If not, I'll go and convert these tests to ^^ format if that looks fine.

[aub]

Yeah, that's a little ugly.... If you can make a function that takes the things you want to change as an argument then that function can itself contain the begin/ensure block:

def change_stuff(what_to_change)
   begin
    apply_changes
    yield
  ensure
    roll_back_changes
  end
end

In your case, I think it would be possible to do this by passing a hash to such a function:

change_stuff(foo: true)

Then the function could loop over the hash and make the requested changes to the Settings. This does seem like it could use some more thought though.... there might be cases where that doesn't work very well.

[knkski]

@aub: Definitely could have went with smaller chunked PRs. The worry I had with that is that we'd get halfway done, get distracted by something urgent and end up with n+1 ways of configuring vets-api.

Adding documentation sounds great, I'll get that added.

[knkski]

@aub: These certs are now residing in a private repo. Do we want to make those available somehow, or just add a note telling external contributors to do something like touch ~/.certs/vetsgov-localhost.crt?

[aub]

Good question. I think it's important that these be private, but I'm not sure what to tell people to do.

[knkski]

@aub: Added with_settings helper function to deal with temporarily changing the Settings object for testing