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

move mysql creds to compose env variables #76

Conversation

jessfraz
Copy link
Contributor

closes #75

@@ -13,6 +13,6 @@
},
"storage": {
"backend": "mysql",
"db_url": "dockercondemo:dockercondemo@tcp(notarymysql:3306)/dockercondemo"
"db_url": "gordontheturtle:dockerrulez@tcp(notarymysql:3306)/development"
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this line be referencing DB_USER et al?

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 believe this is the right format user:password@

Copy link
Contributor

Choose a reason for hiding this comment

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

The format is fine, but the value are hard-coded. What I mean was the connection string should reference the env variables like : "${DB_USER}:${DB_PASS}@"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will that work w cobra we whatever lib you use I'm not familiar

On Jul 17, 2015, at 11:59, Richard Scothern notifications@github.com wrote:

In cmd/notary-server/config.json:

@@ -13,6 +13,6 @@
},
"storage": {
"backend": "mysql",

  •   "db_url": "dockercondemo:dockercondemo@tcp(notarymysql:3306)/dockercondemo"
    
  •   "db_url": "gordontheturtle:dockerrulez@tcp(notarymysql:3306)/development"
    
    The format is fine, but the value are hard-coded. What I mean was the connection string should reference the env variables like : "${DB_USER}:${DB_PASS}@"


Reply to this email directly or view it on GitHub.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree, this should reference the env variables, and they should be setup before this container runs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

does that work w cobra

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ive never used that lib lol

Copy link
Contributor

Choose a reason for hiding this comment

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

I have no idea what Cobra is :)

This thing will be deployed with Ansible, until infra switches to docker-compose, so changes to this file are for now immaterial

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oooooo i see, but in the container i see this being a problem maybe...

On Fri, Jul 17, 2015 at 1:06 PM, Richard Scothern notifications@github.com
wrote:

In cmd/notary-server/config.json
#76 (comment):

@@ -13,6 +13,6 @@
},
"storage": {
"backend": "mysql",

  •   "db_url": "dockercondemo:dockercondemo@tcp(notarymysql:3306)/dockercondemo"
    
  •   "db_url": "gordontheturtle:dockerrulez@tcp(notarymysql:3306)/development"
    

I have no idea what Cobra is :)

This thing will be deployed with Ansible, until infra switches to
docker-compose, so changes to this file are for now immaterial


Reply to this email directly or view it on GitHub
https://github.com/docker/notary/pull/76/files#r34926147.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok ya i think it works

@jessfraz jessfraz force-pushed the 75-move-mysql-creds-to-env-vars branch 2 times, most recently from cd58c9f to 3234fed Compare July 17, 2015 21:23
@jessfraz
Copy link
Contributor Author

buellerrrrrr

ferris

@jessfraz
Copy link
Contributor Author

hi!

@RichardScothern
Copy link
Contributor

ping @endophage

@thaJeztah
Copy link
Contributor

moby/moby#9176 (comment) 🙊👼

@diogomonica
Copy link
Contributor

@thaJeztah the irony is not lost on us :)

The major difference is between doing it/condoning it for everyone using docker.

@thaJeztah
Copy link
Contributor

@diogomonica I know, sorry, couldn't contain myself 😄

(change LGTM)

@endophage
Copy link
Contributor

@RichardScothern Aaron is using a different compose file to deploy (he builds his own images from those we push to hub and deploys those). We want the files as they exist in the repo to be runnable locally so contributors can get up and running easily.

With that in mind, we probably need to revert the changes for ansible.

@RichardScothern
Copy link
Contributor

@endophage I'm aware of this :) What do you want to do with this PR? 🍰

@endophage
Copy link
Contributor

@RichardScothern you personally, nothing, just addressing your earlier comments.

@jessfraz
Copy link
Contributor Author

so can i go back to what i had here originally ;) #76 (diff)

@jessfraz jessfraz force-pushed the 75-move-mysql-creds-to-env-vars branch from 3234fed to 610a64c Compare July 28, 2015 23:13
@jessfraz
Copy link
Contributor Author

updated

@@ -12,6 +12,6 @@
},
"storage": {
"backend": "mysql",
"db_url": "dockercondemo:dockercondemo@tcp(notarymysql:3306)/dockercondemo"
"db_url": "gordontheturtle:dockerrulez@tcp(notarymysql:3306)/development"
Copy link
Contributor

Choose a reason for hiding this comment

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

Indentation looks off here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

having a rough day over here haha

Copy link
Contributor Author

Choose a reason for hiding this comment

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

lol

On Tue, Jul 28, 2015 at 4:17 PM, Sebastiaan van Stijn <
notifications@github.com> wrote:

In cmd/notary-server/config.json
#76 (comment):

@@ -12,6 +12,6 @@
},
"storage": {
"backend": "mysql",

  •   "db_url": "dockercondemo:dockercondemo@tcp(notarymysql:3306)/dockercondemo"
    
  •    "db_url": "gordontheturtle:dockerrulez@tcp(notarymysql:3306)/development"
    

Indentation looks off here


Reply to this email directly or view it on GitHub
https://github.com/docker/notary/pull/76/files#r35713308.

Copy link
Contributor

Choose a reason for hiding this comment

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

I saw, no worries 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

retabbed the whole thing, it was def mixed tabs and spaces :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Thx, looks good now!

@jessfraz jessfraz force-pushed the 75-move-mysql-creds-to-env-vars branch from 610a64c to 17ef432 Compare July 28, 2015 23:24
@diogomonica
Copy link
Contributor

does docker-compose up work with this config? (as in, are we able to point notary to it and it works?)

@jessfraz
Copy link
Contributor Author

yessir

@jessfraz
Copy link
Contributor Author

but you may want to verify for yourself

@jessfraz jessfraz closed this Aug 1, 2015
@jessfraz jessfraz reopened this Aug 5, 2015
Signed-off-by: Jessica Frazelle <princess@docker.com>
@jessfraz jessfraz force-pushed the 75-move-mysql-creds-to-env-vars branch from 17ef432 to 554ea0e Compare August 5, 2015 05:24
@jessfraz
Copy link
Contributor Author

jessfraz commented Aug 5, 2015

rebased..

@jessfraz
Copy link
Contributor Author

come on people

@endophage
Copy link
Contributor

Now that I've been playing with Ansible, there's basically no difference in whether we include the variables in a config file or as ENV variables, they will get defined in the same place then the reefer tool is responsible for either passing them to the process we run or placeholding them into a config . However for local dev, it seems much easier to include them in a config. Thoughts?

@endophage
Copy link
Contributor

@jfrazelle going to close this as it'll be superceded by this PR #185

That PR will allow trust to take full control over deployments once I finish the integration.

@endophage endophage closed this Aug 13, 2015
@jessfraz jessfraz deleted the 75-move-mysql-creds-to-env-vars branch September 3, 2015 21:26
endophage pushed a commit to endophage/notary that referenced this pull request Oct 27, 2015
client: Add optional custom User-Agent to HTTPRemoteStore
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 this pull request may close these issues.

Compose file should not use notarymysql which has hardcoded passwords
7 participants