You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by ahgittin <gi...@git.apache.org> on 2017/09/08 12:10:07 UTC

[GitHub] brooklyn-server pull request #812: use external config in examples

GitHub user ahgittin opened a pull request:

    https://github.com/apache/brooklyn-server/pull/812

    use external config in examples

    some of our tests and examples used plain-text passwords, and this has caused some alarm when people have seen them.  we can say "you can use external but we haven't" though isn't it better if our message is "you *should* use external just like we've shown" ?
    
    this sets up a pre-defined external provider called
    `brooklyn-demo-sample` which defines `hidden-brooklyn-password` as `br00k11n`.
    it can be overridden; PR to follow for docs to explain this.
    
    i think examples elsewhere should be updated to use this wherever a plaintext password is encoded in a blueprint for anything other than the simplest teaching example (clearly caveated and linking to the docs).  i'll do the rest of the stock brooklyn ones shortly.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/ahgittin/brooklyn-server external-config-in-examples

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/brooklyn-server/pull/812.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #812
    
----
commit 5a505e13807f03d71f33f22ba07d3ac55f652a3d
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2017-09-08T12:01:55Z

    add out of box support for `$brooklyn:external("brooklyn-demo-sample", "hidden-brooklyn-password")`
    
    so we can use this in examples rather than a plain-text password

commit c6b41cbb9d0be302bdda15e76785ee59e81b360b
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2017-09-08T12:05:22Z

    update all tests/examples to use externalized password
    
    also tweak mysql config to set up users slightly better

----


---

[GitHub] brooklyn-server pull request #812: use external config in examples

Posted by drigodwin <gi...@git.apache.org>.
Github user drigodwin commented on a diff in the pull request:

    https://github.com/apache/brooklyn-server/pull/812#discussion_r138582530
  
    --- Diff: camp/camp-brooklyn/src/test/resources/visitors-creation-script.sql ---
    @@ -19,18 +19,22 @@
     create database visitors;
     use visitors;
     
    -# not necessary to create user if we grant (and not supported in some dialects)
    -create user 'brooklyn' identified by 'br00k11n';
    -
    -grant usage on *.* to 'brooklyn'@'%' identified by 'br00k11n';
    -
    -# ''@localhost is sometimes set up, overriding brooklyn@'%', so do a second explicit grant
    -grant usage on *.* to 'brooklyn'@'localhost' identified by 'br00k11n';
     
    +# the below will create user (and note create user not supported in some dialects)
    +grant usage on *.* to 'brooklyn'@'%' identified by '${config["creation.script.password"]}';
    --- End diff --
    
    Previously @aledsage deleted [this version](https://github.com/apache/brooklyn-library/blob/master/examples/simple-web-cluster/src/main/resources/visitors-creation-script.sql) of the visitors creation script. We then found out a lot of downstream projects were using it. Do you think it would be worth adding a [default](http://freemarker.org/docs/dgui_template_exp.html#dgui_template_exp_missing_default) here in case that is true for this file?


---

[GitHub] brooklyn-server pull request #812: use external config in examples

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/brooklyn-server/pull/812


---

[GitHub] brooklyn-server issue #812: use external config in examples

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the issue:

    https://github.com/apache/brooklyn-server/pull/812
  
    Good point re examples pointing to master.  Will add password in script.  But we should probably change links in example to point at specific branches too.


---

[GitHub] brooklyn-server issue #812: use external config in examples

Posted by duncangrant <gi...@git.apache.org>.
Github user duncangrant commented on the issue:

    https://github.com/apache/brooklyn-server/pull/812
  
    Should we make it easier to disable the provider?  That way a "live" deployment of amp would prevent it's use.  So if a developer forgets to update their yaml to use a real provider it will fail to deploy.


---

[GitHub] brooklyn-server issue #812: use external config in examples

Posted by m4rkmckenna <gi...@git.apache.org>.
Github user m4rkmckenna commented on the issue:

    https://github.com/apache/brooklyn-server/pull/812
  
    Ignore me ... It was never added. 
    
    But i do think the flow i mentioned above would be incredibly useful



---

[GitHub] brooklyn-server issue #812: use external config in examples

Posted by m4rkmckenna <gi...@git.apache.org>.
Github user m4rkmckenna commented on the issue:

    https://github.com/apache/brooklyn-server/pull/812
  
    @ahgittin I think @neykov added a file based external config provider for this, That enable users to configure for development and then swap to a "proper" config provider. You could just configure it by default



---

[GitHub] brooklyn-server issue #812: use external config in examples

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the issue:

    https://github.com/apache/brooklyn-server/pull/812
  
    note the two other PRs in downstream projects listed above


---

[GitHub] brooklyn-server issue #812: use external config in examples

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the issue:

    https://github.com/apache/brooklyn-server/pull/812
  
    OTOH someone using a live deployment might want to use one of the test blueprints.  The docs describe how to change the password:
    
    https://github.com/apache/brooklyn-docs/pull/209/files#diff-b6c73a383415d543418f8762ce2dbe9eR79
    
    I think that's sufficient.  If someone really wants to disable that key we can tell them to include just the first line there (the provider is present, but empty); if that becomes a popular feature request then we add the ability to completely disable.


---

[GitHub] brooklyn-server issue #812: use external config in examples

Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:

    https://github.com/apache/brooklyn-server/pull/812
  
    @m4rkmckenna there is a "file-based exteranl config supplier" you can use for dev (`PropertiesFileExternalConfigSupplier`), and then switch to Vault or whatever when you're ready.
    
    But I think that @ahgittin's change here is a separate good addition as it means stock examples can work out-of-the-box without the user having to modify `brooklyn.cfg` to configure things like that, or to add the desired password to some file.
    
    ---
    I agree with @drigodwin's suggestion of giving a default in `visitors-creation-script.sql` so that we don't break other people's examples that might be referencing that file. For Brooklyn 1.0.0 we can remove the default, perhaps first warning people on the mailing list.


---