Whirr
  1. Whirr
  2. WHIRR-641

No longer possible to hardcode password for bootstrap user

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 0.8.0
    • Fix Version/s: 0.8.1, 0.9.0
    • Component/s: core
    • Labels:
      None

      Description

      (debating whether to fix this in 0.8.0 with another RC or shrug and accept that it's an edge case and fix it in 0.8.1...)

      With the whirr.template changes added in WHIRR-593, it's no longer possible to specify whirr.bootstrap-user=user:password (or whirr.template=loginUser=user:password, for that matter) to specify login creds for the bootstrap user. This is generally not a problem, since most providers will either use SSH keys or return a generated password, which jclouds picks up. But if for some reason your provider has hard-coded passwords on the images, you're kinda screwed here. The specific change that broke this is https://github.com/apache/whirr/commit/d7865344354b602ad59146afc6eadd7df4b7d344#L9L274.

      1. WHIRR-641-pt2.patch
        1 kB
        Andrew Bayer
      2. WHIRR-641.patch
        19 kB
        Adrian Cole
      3. WHIRR-641.patch
        0.8 kB
        Andrew Bayer

        Activity

        Hide
        Adrian Cole added a comment -

        ahh yes. We did miss this part!!

        +1

        Show
        Adrian Cole added a comment - ahh yes. We did miss this part!! +1
        Hide
        Andrew Bayer added a comment -

        Committed to branch-0.8 as well.

        Show
        Andrew Bayer added a comment - Committed to branch-0.8 as well.
        Hide
        Andrew Bayer added a comment -

        Whoops - forgot my additional local tweak. With this, I'm all good.

        Show
        Andrew Bayer added a comment - Whoops - forgot my additional local tweak. With this, I'm all good.
        Hide
        Andrew Bayer added a comment -

        Committing to trunk now.

        Show
        Andrew Bayer added a comment - Committing to trunk now.
        Hide
        Adrian Cole added a comment -

        +1

        Show
        Adrian Cole added a comment - +1
        Hide
        Andrew Bayer added a comment -

        beta.11 + this patch seems to suffice in my testing, so let's go forward with this patch on trunk.

        Show
        Andrew Bayer added a comment - beta.11 + this patch seems to suffice in my testing, so let's go forward with this patch on trunk.
        Hide
        Adrian Cole added a comment -

        I'd say more correctly "actual problem here is [also] in jclouds". There are syntax errors we've identified and test gaps that are definitely issues. That said, once you clear them out, you are hung until the upstream fixes apply (those in jclouds 1.5.0-beta.11)

        Show
        Adrian Cole added a comment - I'd say more correctly "actual problem here is [also] in jclouds". There are syntax errors we've identified and test gaps that are definitely issues. That said, once you clear them out, you are hung until the upstream fixes apply (those in jclouds 1.5.0-beta.11)
        Hide
        Andrew Bayer added a comment -

        So the actual problem here is in jclouds and is fixed in beta.11, but I think we should go ahead with this patch on trunk (and branch-0.8 once I push the 0.8.0 release this afternoon) anyway. +1.

        Show
        Andrew Bayer added a comment - So the actual problem here is in jclouds and is fixed in beta.11, but I think we should go ahead with this patch on trunk (and branch-0.8 once I push the 0.8.0 release this afternoon) anyway. +1.
        Hide
        Adrian Cole added a comment -

        Can you try this out?

        I think we should include this in 0.8.0

        Show
        Adrian Cole added a comment - Can you try this out? I think we should include this in 0.8.0
        Hide
        Adrian Cole added a comment -

        adds newline per checkstyle

        Show
        Adrian Cole added a comment - adds newline per checkstyle
        Hide
        Adrian Cole added a comment -

        isolated change to where config is parsed. isolated code related to change and backfilled tests

        Show
        Adrian Cole added a comment - isolated change to where config is parsed. isolated code related to change and backfilled tests
        Hide
        Adrian Cole added a comment -

        I offer to write tests for this, and if there's a less dangerous approach to fixing, I'll add that in a revision.

        Show
        Adrian Cole added a comment - I offer to write tests for this, and if there's a less dangerous approach to fixing, I'll add that in a revision.
        Hide
        Adrian Cole added a comment -

        like so many things in whirr. this issue smells of many missing test cases.

        Show
        Adrian Cole added a comment - like so many things in whirr. this issue smells of many missing test cases.
        Hide
        Andrew Bayer added a comment -

        This patch adds back in the (provider).image.login-user override if the TemplateBuilderSpec in ClusterSpec returns non-null getLoginUser() - that gets set to what, historically, has been spec.getBootstrapUser(). I've tested this against our internal cloud setup, which happens to be that edge case where passwords are hardcoded, and it didn't work without this patch...and it does with it.

        So the question is whether this is significant enough to merit a new RC. I'm tending to think it isn't.

        Show
        Andrew Bayer added a comment - This patch adds back in the (provider).image.login-user override if the TemplateBuilderSpec in ClusterSpec returns non-null getLoginUser() - that gets set to what, historically, has been spec.getBootstrapUser(). I've tested this against our internal cloud setup, which happens to be that edge case where passwords are hardcoded, and it didn't work without this patch...and it does with it. So the question is whether this is significant enough to merit a new RC. I'm tending to think it isn't.

          People

          • Assignee:
            Adrian Cole
            Reporter:
            Andrew Bayer
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development