You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by sjcorbett <gi...@git.apache.org> on 2017/12/18 17:10:37 UTC
[GitHub] brooklyn-server pull request #918: Blueprints can include username and passw...
GitHub user sjcorbett opened a pull request:
https://github.com/apache/brooklyn-server/pull/918
Blueprints can include username and password for catalog libraries
For example:
```yaml
brooklyn.catalog:
id: simple-example
version: "1.0"
itemType: template
libraries:
- url: https://nexus.example.com/mybundle.jar
auth:
username: $brooklyn:external("myprovider", "username")
password: $brooklyn:external("myprovider", "password")
item:
...
```
See https://issues.apache.org/jira/browse/BROOKLYN-421 for the motivation.
Changes:
* Moves `Credentials` and `UsernamePassword` from `org.apache.brooklyn.util.http.executor` to `org.apache.brooklyn.util.http.auth`. Deprecated placeholders are left in place in case either has ended up in persisted state.
* Adds `ResourceUtils#getResourceFromUrl(String, Credentials)` to supply credentials when retrieving HTTP resources. The second argument has no effect when the URL's scheme is neither HTTP nor HTTPS.
* Adds `@Nullable Credentials getUrlCredential()` to `OsgiBundleWithUrl`.
* `CatalogItemDtoAbstract#parseLibraries` tests for a map named `auth` containing `username` and `password` keys.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/sjcorbett/brooklyn-server lib-cred
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/brooklyn-server/pull/918.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 #918
----
commit be52a748941a6e181242dc6b87b2722736964fa5
Author: Sam Corbett <sa...@cloudsoftcorp.com>
Date: 2017-12-18T12:21:53Z
ResourceUtils.getResource accepts username and password
Only used for HTTP resources.
commit 47d87053e0e3631872f94afd4cb236552fe8ff9d
Author: Sam Corbett <sa...@cloudsoftcorp.com>
Date: 2017-12-18T12:22:40Z
Blueprints can include username and password for catalog libraries
For example:
brooklyn.catalog:
id: simple-example
version: "1.0"
itemType: template
libraries:
- url: https://nexus.example.com/mybundle.jar
auth:
username: $brooklyn:external("myprovider", "username")
password: $brooklyn:external("myprovider", "password")
item:
...
commit a6c9f71e3fde1daeddb869ed9b9af71fb43ae6a2
Author: Sam Corbett <sa...@cloudsoftcorp.com>
Date: 2017-12-18T15:28:15Z
Move Credentials and UsernamePassword to auth package
----
---
[GitHub] brooklyn-server issue #918: Blueprints can include username and password for...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-server/pull/918
Thanks @sjcorbett - LGTM.
For the record, I tested this in karaf with a toy catalog.bom:
```
brooklyn.catalog:
id: pr918
bundle: pr918
version: 1.0.0-SNAPSHOT
itemType: entity
brooklyn.libraries:
- url: .../brooklyn-etcd-2.7.0-20171219.1516.jar
auth:
username: $brooklyn:external("myexternals", "username")
password: $brooklyn:external("myexternals", "password")
item:
type: io.brooklyn.entity.nosql.etcd.EtcdNode
```
I break-pointed in Brooklyn to check what it was doing. By the time we get into `CatalogItemDtoAbstract.parseLibraries`, looking up the `auth` map, the DSL has already been resolved into the strings pointed at by the username/password. This was earlier than I expected but it's fine.
I checked that the credentials are not persisted (`BasicManagedBundleMemento` doesn't mention credentials, so they won't be).
I check the debug log, searching for my password - it was never logged.
---
[GitHub] brooklyn-server issue #918: Blueprints can include username and password for...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-server/pull/918
Agree with @tbouron that supporting `auth` for ftp would be a nice thing to have in the future, but I don't think it's required for this PR.
By the way @sjcorbett, a pleasure to review - nice clear code, and well tested. Merging now.
---
[GitHub] brooklyn-server pull request #918: Blueprints can include username and passw...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/brooklyn-server/pull/918
---
[GitHub] brooklyn-server pull request #918: Blueprints can include username and passw...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/918#discussion_r158264829
--- Diff: utils/common/src/main/java/org/apache/brooklyn/util/http/executor/UsernamePassword.java ---
@@ -16,28 +16,14 @@
* specific language governing permissions and limitations
* under the License.
*/
-package org.apache.brooklyn.util.http.executor;
-
-import com.google.common.annotations.Beta;
-@Beta
-public class UsernamePassword implements Credentials {
- private final String username;
- private final String password;
+package org.apache.brooklyn.util.http.executor;
+/**
+ * @deprecated since 1.0.0 use {@link org.apache.brooklyn.util.http.auth.UsernamePassword} instead.
--- End diff --
Minor comment I thought of (after merging!) is that it's worth saying why we're leaving this class around (for persisted state backwards compatibility, as well as just normal deprecation policy). We could add something to https://github.com/apache/brooklyn-server/blob/master/core/src/main/resources/org/apache/brooklyn/core/mgmt/persist/deserializingClassRenames.properties, to support future deletion of this class (I don't think we want it deleted without adding a line there).
---
[GitHub] brooklyn-server pull request #918: Blueprints can include username and passw...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/918#discussion_r158264960
--- Diff: utils/common/src/main/java/org/apache/brooklyn/util/http/executor/UsernamePassword.java ---
@@ -16,28 +16,14 @@
* specific language governing permissions and limitations
* under the License.
*/
-package org.apache.brooklyn.util.http.executor;
-
-import com.google.common.annotations.Beta;
-@Beta
-public class UsernamePassword implements Credentials {
- private final String username;
- private final String password;
+package org.apache.brooklyn.util.http.executor;
+/**
+ * @deprecated since 1.0.0 use {@link org.apache.brooklyn.util.http.auth.UsernamePassword} instead.
+ */
+public class UsernamePassword extends org.apache.brooklyn.util.http.auth.UsernamePassword {
--- End diff --
Should this also implement `org.apache.brooklyn.util.http.executor.Credentials`, for backwards compatibility?
---