Skip to content

Allow local vcap options to be supplied in a file, not just an object #31

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

Closed
jsmoorman opened this issue Apr 3, 2018 · 7 comments · Fixed by #33
Closed

Allow local vcap options to be supplied in a file, not just an object #31

jsmoorman opened this issue Apr 3, 2018 · 7 comments · Fixed by #33

Comments

@jsmoorman
Copy link

jsmoorman commented Apr 3, 2018

Would it be possible to load everything defined under env: in the manifest.yml to be loaded into process.env in the constructor when isLocal is true?

This would allow me to access my environment variables defined in the manifest.yml without having to use another library or any additional code.

@pmuellr
Copy link
Member

pmuellr commented Apr 5, 2018

That sounds interesting, but I think I'll have to noodle on it.

I'm hesitant to reset env vars magically, since manifest.yml is currently only used in "local" mode, and only to get the app name. To use values from the manifest more widely, we'd have to make it an opt-in thing, otherwise it would break existing apps.

I think if that functionality were to be added, it would have to be opt-in, like a new boolean option on getAppEnv() called something like setManifestEnvIfLocal or such.

@jsmoorman
Copy link
Author

Thank you for the response.

Upon further work with Cloud Foundry and this module, I started using user-defined services to store my credentials instead of the env variables. But this now presents another problem.

I understand how to read my credentials locally by passing

{
  vcap: {
    services: {
      "user-provided": [
        {
          "credentials": {
            "password": "<password>",
            "username": "<username>"
          },
          "name": "<custom-service>",
        }
      ]
    }
  }
}

to cfenv.getAppEnv(), but I run into the issue of having to either:

  • hardcode these options and include the secrets through process.env
  • read in a custom .json

I like the second bullet but I dislike having to either wrap the require('<filename>.json') in a try/catch like you suggest in a comment you left in #6 or pushing this untracked file to Cloud Foundry so the app doesn't complain it can't find it only to ignore it anyways.

Is there a way to allow the options parameter passed to getAppEnv() to also be a string, and when it is, it's assumed to be a path to a .json that can be parsed when running locally?

@pmuellr
Copy link
Member

pmuellr commented Apr 5, 2018

So maybe something like a vcapFile property on the getAppEnv() options object, which would be a filename of a JSON file with application and services properties, and only read and used when running locally?

That sounds fairly reasonable.

@pmuellr pmuellr changed the title Access manifest.yml env Allow local vcap options to be supplied in a file, not just an object Apr 18, 2018
pmuellr pushed a commit that referenced this issue Apr 18, 2018
@pmuellr
Copy link
Member

pmuellr commented Apr 18, 2018

@jsmoorman I have an implementation of this in PR #33 ; see notes there about testing with this implementation. I'd like to find out if this works for you.

@jsmoorman
Copy link
Author

So I've just tested it and it works amazingly!

I've exported the VCAP_SERVICES from my Cloud Foundry App on Bluemix and just wrapped it with:

{
  "services": "<exported VCAP_SERVICES>"
}

The only possible suggestion I have is to allow users to use the exported json itself as the VCAP_SERVICES instead of as the whole app environment so that users don't have to wrap the export as I did above -- but then you would lose the ability to customize the VCAP_APPLICATION.

That is, unless you allowed them to use it in tandem with the vcap field in the options argument, but then you have the possibility of users providing VCAP_SERVICES through the vcapFile and through the vcap fields.

@jsmoorman
Copy link
Author

But to answer your original question, this works exactly as expected and provides the functionality I needed.

Thank you!

@pmuellr
Copy link
Member

pmuellr commented Apr 19, 2018

The only possible suggestion I have is to allow users to use the exported json itself as the VCAP_SERVICES instead of as the whole app environment so that users don't have to wrap the export as I did above

Ya, I can see that, but I think only if such a 'services-only' JSON file would be useful in some other environment. Otherwise, it's kinda a one-off thing anyway, so I don't see the extra services property indirection as too bad.

It would also not be parallel to the existing vcap option.

Probably worth noodling on whether there should be vcapServices and vcapServicesFile options that would just have the "services" bits and not "application" bits, especially if it turns out the application bits rarely need to be set like this given the default processing.

But let me go ahead and publish what I have now, I think this weekend or next week. Tied up for now and I want to have some time ready if for some unknown reason this causes existing users problems, so I can fix that.

Thanks a bunch for trying this out!

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 a pull request may close this issue.

2 participants