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

keyFilename makes 12-factor app style deployment difficult #136

Closed
hulbert opened this issue Aug 26, 2014 · 11 comments
Closed

keyFilename makes 12-factor app style deployment difficult #136

hulbert opened this issue Aug 26, 2014 · 11 comments
Assignees
Labels
🚨 This issue needs some love. triage me I really want to be triaged. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Milestone

Comments

@hulbert
Copy link

hulbert commented Aug 26, 2014

I'm trying to use cloud storage which the documentation says I should initialize as such:

var gcloud = require('gcloud'),
    bucket = new gcloud.storage.Bucket({
        bucketName: YOUR_BUCKET_NAME,
        keyFilename: '/path/to/the/key.json'
    });

I dug into the code and this keyFilename option seems to be the only way to specify this required info, and it expects a filepath string. This makes storing the key's JSON as an environmental variable pretty complicated since I'd have to write the env value to a file, then pass the path to gcloud.

This seems to be a similar issue someone faced with Google APIs, a key file, and Heroku: http://ar.zu.my/how-to-store-private-key-files-in-heroku/

As he outlines, it seems there are four options, none super simple:

  1. Include the private key in my git repository.
  2. Store it in S3, and pull it when needed.
  3. Use custom build-pack which will include the key.
  4. Store it as config vars.

Even just having the option to pass this information in as a Javascript object (which makes sense for a node library) would be nice.

@rakyll
Copy link
Contributor

rakyll commented Aug 26, 2014

We should never assume that key file will be read from disk. We should require it to be passed as a plain object. Any other opinions?

@rakyll rakyll added this to the M1: core, datastore & storage milestone Aug 26, 2014
@rakyll rakyll added the bug label Aug 26, 2014
@silvolu
Copy link
Contributor

silvolu commented Aug 26, 2014

I would add a key option to pass the object, but keep the keyFilename is as well.
Some people like the 'download the key and pass the path to the constructor'™ approach, mostly in the getting started experience context.

@rakyll
Copy link
Contributor

rakyll commented Aug 26, 2014

SGTM as we discussed offline.

@ryanseys
Copy link
Contributor

Yes, both options is best. keyFilename: 'path/goes/here.json' should more or less just be an alias for key: require('path/goes/here.json') where key accepts a plain JavaScript object.

@rakyll
Copy link
Contributor

rakyll commented Aug 26, 2014

keyFilename should work the way it works (without require), otherwise it becomes impossible to specify relative paths to keyFilename. See 7641028.

@ryanseys
Copy link
Contributor

Interesting... I've never had issues with relative paths + require but I've never pushed it to its limits either. 😄

fs.readFile is definitely better anyway because it's async and won't automagically cache. 👍

@hulbert
Copy link
Author

hulbert commented Aug 26, 2014

Just an end-user perspective/question, is the entire JSON really needed or is it just using the private_key value? If you were to add a key option I would potentially be confused if this is to the whole object or just the value specified for the key private_key in the downloaded JSON file.

This confusion might be worsened in my example of using environmental variables as I would likely break the JSON file into four ENV variables like so:

declared environmental vars:

GOOGLE_CLOUD_private_key_id=value
GOOGLE_CLOUD_private_key=value
GOOGLE_CLOUD_client_email=value
GOOGLE_CLOUD_client_id=value
GOOGLE_CLOUD_type=value

Initializing gcloud-node:

var gcloud = require('gcloud'),
    bucket = new gcloud.storage.Bucket({
        bucketName: YOUR_BUCKET_NAME,
        key: {
            "private_key_id": process.env.GOOGLE_CLOUD_private_key_id,
            "private_key": process.env.GOOGLE_CLOUD_private_key,
            "client_email": process.env.GOOGLE_CLOUD_client_email,
            "client_id": process.env.GOOGLE_CLOUD_client_id,
            "type": process.env.GOOGLE_CLOUD_type
        }
    });

@rakyll
Copy link
Contributor

rakyll commented Aug 26, 2014

We can rename the key to credentials which is basically what Connection object is doing at the moment. It's reading the file from keyFilename, parse and set it to Connection.prototype.credentials.

Btw, you only need to provide private_key and client_email.

var gcloud = require('gcloud'),
    bucket = new gcloud.storage.Bucket({
        bucketName: YOUR_BUCKET_NAME,
        credentials: {
            "private_key": process.env.GOOGLE_CLOUD_private_key,
            "client_email": process.env.GOOGLE_CLOUD_client_email,
        }
    });

@hulbert
Copy link
Author

hulbert commented Aug 27, 2014

@rakyll 👍 your example would address my original issue

stephenplusplus added a commit to stephenplusplus/gcloud-node that referenced this issue Aug 27, 2014
stephenplusplus added a commit to stephenplusplus/gcloud-node that referenced this issue Aug 27, 2014
rakyll pushed a commit that referenced this issue Aug 27, 2014
connection: allow passing a credentials object. fixes #136.
@jgeewax jgeewax added the auth label Feb 2, 2015
@jgeewax jgeewax modified the milestones: M1: core, datastore & storage, Core Stable Feb 2, 2015
miro added a commit to miro/dakdak-api that referenced this issue Jan 9, 2016
@luis-agm
Copy link

I'm sorry but is this somewhere on the docs?

@hulbert
Copy link
Author

hulbert commented Nov 21, 2017

@luis-agm it is, if you see the example here you can see they mention the credentials object: https://github.com/GoogleCloudPlatform/google-cloud-node/blob/master/README.md#elsewhere

sofisl pushed a commit that referenced this issue Nov 11, 2022
…ript generator. (#136)

Also removing the explicit generator tag for the IAMPolicy mixin for the kms and pubsub APIS as the generator will now read it from the .yaml file.

PiperOrigin-RevId: 385101839

Source-Link: googleapis/googleapis@80f4042

Source-Link: googleapis/googleapis-gen@d3509d2
sofisl pushed a commit that referenced this issue Nov 11, 2022
sofisl pushed a commit that referenced this issue Nov 11, 2022
Source-Link: googleapis/synthtool@d229a12
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-nodejs:latest@sha256:74ab2b3c71ef27e6d8b69b1d0a0c9d31447777b79ac3cd4be82c265b45f37e5e
sofisl pushed a commit that referenced this issue Nov 11, 2022
sofisl pushed a commit that referenced this issue Nov 11, 2022
#136)

* build: update auto-approve file to v2

Source-Link: googleapis/synthtool@19eb6fc
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-nodejs:latest@sha256:b9e4584a1fe3c749e3c37c92497b13dce653b2e694f0261f0610eb0e15941357

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
sofisl pushed a commit that referenced this issue Nov 11, 2022
See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
Co-authored-by: Benjamin E. Coe <bencoe@google.com>
sofisl pushed a commit that referenced this issue Nov 11, 2022
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [jsdoc](https://togithub.com/jsdoc/jsdoc) | [`^3.6.6` -> `^4.0.0`](https://renovatebot.com/diffs/npm/jsdoc/3.6.11/4.0.0) | [![age](https://badges.renovateapi.com/packages/npm/jsdoc/4.0.0/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/npm/jsdoc/4.0.0/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/npm/jsdoc/4.0.0/compatibility-slim/3.6.11)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/npm/jsdoc/4.0.0/confidence-slim/3.6.11)](https://docs.renovatebot.com/merge-confidence/) |

---

### Release Notes

<details>
<summary>jsdoc/jsdoc</summary>

### [`v4.0.0`](https://togithub.com/jsdoc/jsdoc/blob/HEAD/CHANGES.md#&#8203;400-November-2022)

[Compare Source](https://togithub.com/jsdoc/jsdoc/compare/3.6.11...084218523a7d69fec14a852ce680f374f526af28)

-   JSDoc releases now use [semantic versioning](https://semver.org/). If JSDoc makes
    backwards-incompatible changes in the future, the major version will be incremented.
-   JSDoc no longer uses the [`taffydb`](https://taffydb.com/) package. If your JSDoc template or
    plugin uses the `taffydb` package, see the
    [instructions for replacing `taffydb` with `@jsdoc/salty`](https://togithub.com/jsdoc/jsdoc/tree/main/packages/jsdoc-salty#use-salty-in-a-jsdoc-template).
-   JSDoc now supports Node.js 12.0.0 and later.

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "after 9am and before 3pm" (UTC), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://app.renovatebot.com/dashboard#github/googleapis/nodejs-org-policy).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC4xNy4xIiwidXBkYXRlZEluVmVyIjoiMzQuMTcuMSJ9-->
sofisl pushed a commit that referenced this issue Nov 16, 2022
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [jsdoc-region-tag](https://togithub.com/googleapis/jsdoc-region-tag) | [`^1.0.6` -> `^2.0.0`](https://renovatebot.com/diffs/npm/jsdoc-region-tag/1.3.1/2.0.0) | [![age](https://badges.renovateapi.com/packages/npm/jsdoc-region-tag/2.0.0/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/npm/jsdoc-region-tag/2.0.0/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/npm/jsdoc-region-tag/2.0.0/compatibility-slim/1.3.1)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/npm/jsdoc-region-tag/2.0.0/confidence-slim/1.3.1)](https://docs.renovatebot.com/merge-confidence/) |

---

### Release Notes

<details>
<summary>googleapis/jsdoc-region-tag</summary>

### [`v2.0.0`](https://togithub.com/googleapis/jsdoc-region-tag/blob/HEAD/CHANGELOG.md#&#8203;200-httpsgithubcomgoogleapisjsdoc-region-tagcomparev131v200-2022-05-20)

[Compare Source](https://togithub.com/googleapis/jsdoc-region-tag/compare/v1.3.1...v2.0.0)

##### ⚠ BREAKING CHANGES

-   update library to use Node 12 ([#&#8203;107](https://togithub.com/googleapis/jsdoc-region-tag/issues/107))

##### Build System

-   update library to use Node 12 ([#&#8203;107](https://togithub.com/googleapis/jsdoc-region-tag/issues/107)) ([5b51796](https://togithub.com/googleapis/jsdoc-region-tag/commit/5b51796771984cf8b978990025f14faa03c19923))

##### [1.3.1](https://www.github.com/googleapis/jsdoc-region-tag/compare/v1.3.0...v1.3.1) (2021-08-11)

##### Bug Fixes

-   **build:** migrate to using main branch ([#&#8203;79](https://www.togithub.com/googleapis/jsdoc-region-tag/issues/79)) ([5050615](https://www.github.com/googleapis/jsdoc-region-tag/commit/50506150b7758592df5e389c6a5c3d82b3b20881))

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "after 9am and before 3pm" (UTC), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox.

---

This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://app.renovatebot.com/dashboard#github/googleapis/nodejs-managed-identities).
sofisl pushed a commit that referenced this issue Nov 18, 2022
* updated CHANGELOG.md [ci skip]

* updated package.json [ci skip]

* updated samples/package.json [ci skip]
sofisl pushed a commit that referenced this issue Nov 30, 2022
This PR was generated using Autosynth. 🌈


<details><summary>Log from Synthtool</summary>

```
2020-03-22 04:20:26,291 synthtool > Executing /tmpfs/src/git/autosynth/working_repo/synth.py.
2020-03-22 04:20:26,344 synthtool > Ensuring dependencies.
2020-03-22 04:20:26,349 synthtool > Cloning googleapis.
2020-03-22 04:20:27,048 synthtool > Pulling Docker image: gapic-generator-typescript:latest
latest: Pulling from gapic-images/gapic-generator-typescript
Digest: sha256:3762b8bcba247ef4d020ffc7043e2881a20b5fab0ffd98d542f365d3f3a3829d
Status: Image is up to date for gcr.io/gapic-images/gapic-generator-typescript:latest
2020-03-22 04:20:27,975 synthtool > Generating code for: google/cloud/datacatalog/v1beta1.
2020-03-22 04:20:29,239 synthtool > Generated code into /tmpfs/tmp/tmp3v_r80rn.
.eslintignore
.eslintrc.yml
.github/ISSUE_TEMPLATE/bug_report.md
.github/ISSUE_TEMPLATE/feature_request.md
.github/ISSUE_TEMPLATE/support_request.md
.github/PULL_REQUEST_TEMPLATE.md
.github/publish.yml
.github/release-please.yml
.github/workflows/ci.yaml
.kokoro/common.cfg
.kokoro/continuous/node10/common.cfg
.kokoro/continuous/node10/docs.cfg
.kokoro/continuous/node10/lint.cfg
.kokoro/continuous/node10/samples-test.cfg
.kokoro/continuous/node10/system-test.cfg
.kokoro/continuous/node10/test.cfg
.kokoro/continuous/node12/common.cfg
.kokoro/continuous/node12/test.cfg
.kokoro/continuous/node8/common.cfg
.kokoro/continuous/node8/test.cfg
.kokoro/docs.sh
.kokoro/lint.sh
.kokoro/presubmit/node10/common.cfg
.kokoro/presubmit/node10/docs.cfg
.kokoro/presubmit/node10/lint.cfg
.kokoro/presubmit/node10/samples-test.cfg
.kokoro/presubmit/node10/system-test.cfg
.kokoro/presubmit/node10/test.cfg
.kokoro/presubmit/node12/common.cfg
.kokoro/presubmit/node12/test.cfg
.kokoro/presubmit/node8/common.cfg
.kokoro/presubmit/node8/test.cfg
.kokoro/presubmit/windows/common.cfg
.kokoro/presubmit/windows/test.cfg
.kokoro/publish.sh
.kokoro/release/docs.cfg
.kokoro/release/docs.sh
.kokoro/release/publish.cfg
.kokoro/samples-test.sh
.kokoro/system-test.sh
.kokoro/test.bat
.kokoro/test.sh
.kokoro/trampoline.sh
.mocharc.js
.nycrc
.prettierignore
.prettierrc
CODE_OF_CONDUCT.md
CONTRIBUTING.md
LICENSE
README.md
codecov.yaml
renovate.json
Skipping: samples/README.md
2020-03-22 04:20:29,951 synthtool > Replaced '/data-catalog/docs/' in src/v1beta1/data_catalog_client.ts.
2020-03-22 04:20:29,952 synthtool > Replaced '/compute/docs/regions-zones/' in src/v1beta1/data_catalog_client.ts.
npm WARN npm npm does not support Node.js v12.16.1
npm WARN npm You should probably upgrade to a newer version of node as we
npm WARN npm can't make any promises that npm will work with this version.
npm WARN npm Supported releases of Node.js are the latest release of 6, 8, 9, 10, 11.
npm WARN npm You can find the latest version at https://nodejs.org/

> protobufjs@6.8.9 postinstall /tmpfs/src/git/autosynth/working_repo/node_modules/protobufjs
> node scripts/postinstall


> @google-cloud/datacatalog@1.8.0 prepare /tmpfs/src/git/autosynth/working_repo
> npm run compile

npm WARN npm npm does not support Node.js v12.16.1
npm WARN npm You should probably upgrade to a newer version of node as we
npm WARN npm can't make any promises that npm will work with this version.
npm WARN npm Supported releases of Node.js are the latest release of 6, 8, 9, 10, 11.
npm WARN npm You can find the latest version at https://nodejs.org/

> @google-cloud/datacatalog@1.8.0 compile /tmpfs/src/git/autosynth/working_repo
> tsc -p . && cp -r protos build/

npm notice created a lockfile as package-lock.json. You should commit this file.
npm WARN optional SKIPPING OPTIONAL DEPENDENCY: fsevents@~2.1.1 (node_modules/chokidar/node_modules/fsevents):
npm WARN notsup SKIPPING OPTIONAL DEPENDENCY: Unsupported platform for fsevents@2.1.2: wanted {"os":"darwin","arch":"any"} (current: {"os":"linux","arch":"x64"})

added 605 packages from 344 contributors and audited 1418 packages in 17.022s
found 0 vulnerabilities

npm WARN npm npm does not support Node.js v12.16.1
npm WARN npm You should probably upgrade to a newer version of node as we
npm WARN npm can't make any promises that npm will work with this version.
npm WARN npm Supported releases of Node.js are the latest release of 6, 8, 9, 10, 11.
npm WARN npm You can find the latest version at https://nodejs.org/

> @google-cloud/datacatalog@1.8.0 fix /tmpfs/src/git/autosynth/working_repo
> gts fix && eslint . --fix

installing semver@^5.5.0
installing uglify-js@^3.3.25
installing espree@^3.5.4
installing escodegen@^1.9.1
2020-03-22 04:21:06,067 synthtool > Wrote metadata to synth.metadata.

```
</details>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🚨 This issue needs some love. triage me I really want to be triaged. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

No branches or pull requests

8 participants