You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Svet <no...@github.com> on 2017/07/12 05:09:39 UTC
[jclouds/jclouds-labs] Generate Azure VM password on the fly (#402)
Used to be a fixed password which anyone can use to login to the newly provisioned machines.
You can view, comment on, or merge this pull request online at:
https://github.com/jclouds/jclouds-labs/pull/402
-- Commit Summary --
* Generate Azure VM password on the fly
-- File Changes --
M azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/AzureComputeProviderMetadata.java (5)
M azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/compute/AzureComputeServiceAdapter.java (12)
M azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/compute/strategy/CreateResourcesThenCreateNodes.java (21)
A azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/util/Passwords.java (32)
-- Patch Links --
https://github.com/jclouds/jclouds-labs/pull/402.patch
https://github.com/jclouds/jclouds-labs/pull/402.diff
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/402
Re: [jclouds/jclouds-labs] Generate Azure VM password on the fly
(#402)
Posted by Svet <no...@github.com>.
Thanks @nacx, merging after build completes.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/402#issuecomment-314746485
Re: [jclouds/jclouds-labs] Generate Azure VM password on the fly
(#402)
Posted by Svet <no...@github.com>.
neykov commented on this pull request.
> + * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.jclouds.azurecompute.arm.util;
+
+import com.google.common.io.BaseEncoding;
+
+import java.util.Random;
+
+// Seems to be a common theme between providers, perhaps should be provided by core (see other 'Passwords' classes)
+public class Passwords {
+1 to refactor and add tests. I can give it a try.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/402#discussion_r127899519
Re: [jclouds/jclouds-labs] Generate Azure VM password on the fly
(#402)
Posted by Ignasi Barrera <no...@github.com>.
nacx commented on this pull request.
> @@ -130,6 +136,20 @@ protected CreateResourcesThenCreateNodes(
return super.execute(group, count, template, goodNodes, badNodes, customizationResponses);
}
+ // Azure requires that we pass it the VM password. Need to generate one if not overriden by the user.
+ private void generatePasswordIfNoneProvided(Template template) {
+ TemplateOptions options = template.getOptions();
+ if (options.getLoginPassword() == null) {
+ Optional<String> passwordOptional = template.getImage().getDefaultCredentials().getOptionalPassword();
+ if (passwordOptional.isPresent()) {
It's only a syntactic sugar. You have also the supplier option, but it is not that readable then. I'm OK with keeping unchanged :) Feel free to merge your preferred version.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/402#discussion_r126933825
Re: [jclouds/jclouds-labs] Generate Azure VM password on the fly
(#402)
Posted by Ignasi Barrera <no...@github.com>.
nacx commented on this pull request.
> + * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.jclouds.azurecompute.arm.util;
+
+import com.google.common.io.BaseEncoding;
+
+import java.util.Random;
+
+// Seems to be a common theme between providers, perhaps should be provided by core (see other 'Passwords' classes)
+public class Passwords {
Sure they are. For me, it would make sense to refactor this class out to jclouds-compute and provide several generic methods to generate passwords, with their corresponding tests. Providers should be able to use the ones that match their policies, but this class is definitely a good fit for jclouds-compute. Users would be able to easily use it too in a portable way.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/402#discussion_r127891342
Re: [jclouds/jclouds-labs] Generate Azure VM password on the fly
(#402)
Posted by Svet <no...@github.com>.
Merged to [master](https://git1-us-west.apache.org/repos/asf?p=jclouds-labs.git;a=commit;h=3641cdb44c192be381f82d8886f966c248474f4d) and [2.0.x](https://git1-us-west.apache.org/repos/asf?p=jclouds-labs.git;a=commit;h=f27b5f944887bdefba78d7bbd339ea68db76725c)
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/402#issuecomment-315747187
Re: [jclouds/jclouds-labs] Generate Azure VM password on the fly
(#402)
Posted by Svet <no...@github.com>.
Closed #402.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/402#event-1166369934
Re: [jclouds/jclouds-labs] Generate Azure VM password on the fly
(#402)
Posted by Andrew Phillips <no...@github.com>.
demobox commented on this pull request.
> + * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.jclouds.azurecompute.arm.util;
+
+import com.google.common.io.BaseEncoding;
+
+import java.util.Random;
+
+// Seems to be a common theme between providers, perhaps should be provided by core (see other 'Passwords' classes)
+public class Passwords {
@neykov @nacx Are there any requirements around the value produced by this class (length, allowed characters) etc.? If so, should we add a small test to verify those?
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/402#pullrequestreview-50485264
Re: [jclouds/jclouds-labs] Generate Azure VM password on the fly
(#402)
Posted by Svet <no...@github.com>.
neykov commented on this pull request.
> + *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.jclouds.azurecompute.arm.util;
+
+import com.google.common.io.BaseEncoding;
+
+import java.util.Random;
+
+// Seems to be a common theme between providers, perhaps should be provided by core (see other 'Passwords' classes)
Will leave that for another PR.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/402#discussion_r126933309
Re: [jclouds/jclouds-labs] Generate Azure VM password on the fly
(#402)
Posted by Svet <no...@github.com>.
neykov commented on this pull request.
> @@ -130,6 +136,20 @@ protected CreateResourcesThenCreateNodes(
return super.execute(group, count, template, goodNodes, badNodes, customizationResponses);
}
+ // Azure requires that we pass it the VM password. Need to generate one if not overriden by the user.
+ private void generatePasswordIfNoneProvided(Template template) {
+ TemplateOptions options = template.getOptions();
+ if (options.getLoginPassword() == null) {
+ Optional<String> passwordOptional = template.getImage().getDefaultCredentials().getOptionalPassword();
+ if (passwordOptional.isPresent()) {
Didn't like it initially because it will generate a password regardless of whether used. But +1 for cleaner code. Updated.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/402#discussion_r126933224
Re: [jclouds/jclouds-labs] Generate Azure VM password on the fly
(#402)
Posted by Ignasi Barrera <no...@github.com>.
nacx approved this pull request.
> @@ -130,6 +136,20 @@ protected CreateResourcesThenCreateNodes(
return super.execute(group, count, template, goodNodes, badNodes, customizationResponses);
}
+ // Azure requires that we pass it the VM password. Need to generate one if not overriden by the user.
+ private void generatePasswordIfNoneProvided(Template template) {
+ TemplateOptions options = template.getOptions();
+ if (options.getLoginPassword() == null) {
+ Optional<String> passwordOptional = template.getImage().getDefaultCredentials().getOptionalPassword();
+ if (passwordOptional.isPresent()) {
More concise version: `options.overrideLoginPassword(passwordOptional.or(Passwords.generate()))`.
> + *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.jclouds.azurecompute.arm.util;
+
+import com.google.common.io.BaseEncoding;
+
+import java.util.Random;
+
+// Seems to be a common theme between providers, perhaps should be provided by core (see other 'Passwords' classes)
+1 to refactor all existing `Password` util classes into something reusable and moving it to compute-core. Something like we have for dates, for example, where we have a class that provides access to different date formatters. We could have a Passwords utils class that provided access to a set of preconfigured password conventions.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/402#pullrequestreview-49458145