Skip to content
This repository has been archived by the owner on Jan 8, 2024. It is now read-only.

Feat/docker auth #2895

Merged
merged 22 commits into from
Feb 9, 2022
Merged

Feat/docker auth #2895

merged 22 commits into from
Feb 9, 2022

Conversation

paladin-devops
Copy link
Contributor

@paladin-devops paladin-devops commented Jan 12, 2022

Fixes #2577.

Uses AuthConfig type from Docker's Go API in order to authenticate to a private registry when an image is built. This is required if a base image is from a private repo.

Example build stanza with username/password auth configured:

  build {
    use "docker" {
      auth {
        hostname       = "registry.hub.docker.com"
        username       = "dockeruser"
        password       = "dockerpass"
      }
    }
  }

All of the following parameters are supported (map of string):

        hostname      = ""
        username      = ""
        password      = ""
        email         = ""
        auth          = ""
        serverAddress = ""
        identityToken = ""
        registryToken = ""

Copy link
Contributor

@evanphx evanphx left a comment

Choose a reason for hiding this comment

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

Looks fine other than using a struct for Auth. Could you also rebase the PR?

builtin/docker/builder.go Outdated Show resolved Hide resolved
@paladin-devops
Copy link
Contributor Author

@evanphx thanks for the feedback! Updates are made and PR is rebased.

@briancain briancain requested review from evanphx and a team January 13, 2022 17:39
builtin/docker/builder.go Show resolved Hide resolved
@evanphx
Copy link
Contributor

evanphx commented Jan 19, 2022

@paladin-devops totally right, I missed Hostname.

So let me pose a larger question to ya: would you be willing to extend this authentication setup to all the other contexts that Docker is accessed? That is one element that we need to cleanup anyway (and one that you very well might hit as well). Basically, build/registry/platform and pull/build should all use the same authentication scheme, where scheme I mean that the auth information is specified in the same way (same configuration variable names, etc).

@paladin-devops
Copy link
Contributor Author

@evanphx I'd be willing to take that on! As I was developing this I also felt like it'd be a nicer UX for the auth to be consistent wherever Docker is used, and have more "user friendly" input options.

@evanphx
Copy link
Contributor

evanphx commented Jan 19, 2022

@paladin-devops Super! Yes, please take a look at the referenced plugin components and see about making the have a common auth mechanism. Thanks!

@paladin-devops
Copy link
Contributor Author

paladin-devops commented Jan 20, 2022

After updating auth for other contexts of Docker within Waypoint, this waypoint.hcl shows the same auth configuration being used for each.

project = "lab"

app "docker-auth-test" {
  build {
    use "docker" {
      auth {
        hostname = "registry.hub.docker.com"
        username = "username"
        password = "password"
      }
    }

    workspace "pull" {
      use "docker-pull" {
        image = "paladindevops/alpine"
        tag = "very-latest"
        auth {
          username = "username"
          password = "password"
        }
      }
    }

    registry {
      use "docker" {
        image = "paladindevops/alpine"
        tag   = "very-latest"
        auth {
          username = "username"
          password = "password"
        }
      }
    }
  }
  deploy {
    use "docker" {
      auth {
        username = "username"
        password = "password"
      }
    }
  }
}

@paladin-devops
Copy link
Contributor Author

@evanphx I've updated the documentation as well. I intentionally left out the Email option (deprecated) and the Auth option (I can't find any documentation on its purpose). As per the struct, though, they are still allowed values since the API supports them, but didn't want to advertise deprecated/unknown inputs.

Refs:

@evanphx
Copy link
Contributor

evanphx commented Jan 20, 2022

@paladin-devops So I think I realized one reason I was confused about the hostname: In the registry build case, it's the hostname embedded in the image name that governs where the auth info goes. So maybe error out if they specify a Hostname to the auth block in registry.

Copy link
Contributor

@evanphx evanphx left a comment

Choose a reason for hiding this comment

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

So we need to still support EncodedAuth and probably at a minimum, move the Auth{} to the right hand side of those empty checks.

builtin/docker/builder.go Outdated Show resolved Hide resolved
builtin/docker/pull/builder.go Outdated Show resolved Hide resolved
@paladin-devops
Copy link
Contributor Author

paladin-devops commented Jan 22, 2022

@evanphx this waypoint.hcl demonstrates the usage of encoded_auth alongside the new auth block for the puller and registry Docker plugins so that the old and new methods of auth are supported for both. Additionally the hostname-test workspace shows that it will error out if the user provides the hostname in the registry block.

  build {
    //default builder uses auth
    use "docker" {
      auth {
        hostname = "registry.hub.docker.com"
        username = "username"
        password = "password"
      }
    }

    //builder, private repo base image, auth provided - expected success
    workspace "builder-auth" {
      use "docker" {
        auth {
          hostname = "registry.hub.docker.com"
          username = "username"
          password = "password"
        }
      }
    }

    //builder, private repo base image, auth not provided - expected failure: authentication error
    workspace "builder-noauth" {
      use "docker" {}
    }

    //puller, auth used for private repo - expected success
    workspace "pull-auth" {
      use "docker-pull" {
        image = "paladindevops/alpine"
        tag   = "very-latest"
        auth {
          username = "username"
          password = "password"
        }
      }
    }

    //puller, encoded_auth used for private repo - expected success
    workspace "pull-encoded" {
      use "docker-pull" {
        image = "paladindevops/alpine"
        tag = "very-latest"
        encoded_auth = "encoded_auth_string"
      }
    }

    //puller, neither auth nor encoded_auth used for private repo - expected failure: authentication error
    workspace "pull-noauth" {
      use "docker-pull" {
        image = "paladindevops/alpine"
        tag = "very-latest"
      }
    }

    //puller, no auth used for public repo - expected success (no auth required)
    workspace "pull-public" {
      use "docker-pull" {
        image = "alpine"
        tag = "latest"
      }
    }

    registry {
      //default registry uses auth
      use "docker" {
        image = "paladindevops/alpine"
        tag   = "very-latest"
        auth {
          username = "username"
          password = "password"
        }
      }

      //auth used for push to private repo - expected success
      workspace "pull-auth" {
        use "docker" {
          image = "paladindevops/alpine"
          tag   = "very-latest"
          auth {
            username = "username"
            password = "password"
          }
        }
      }
      
      //encoded_auth used for private repo - expected success
      workspace "pull-encoded" {
        use "docker" {
          image = "paladindevops/alpine"
          tag   = "very-latest"
          encoded_auth = "encoded_auth_string"
        }
      }

      //auth used for private repo, hostname provided - expected failure: hostname not permitted in registry auth
      workspace "hostname-test" {
        use "docker" {
          image = "paladindevops/alpine"
          tag   = "very-latest"
          auth {
            hostname = "registry.hub.docker.com"
            username = "username"
            password = "password"
          }
        }
      }
    }
  }

For reference, my Dockerfile with a base image in a private repo:

FROM registry.hub.docker.com/paladindevops/alpine:latest

@paladin-devops paladin-devops changed the title Feat/docker builder auth Feat/docker auth Jan 24, 2022
Copy link
Contributor

@evanphx evanphx left a comment

Choose a reason for hiding this comment

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

Looks good! Could you resolve the merge conflicts?

@evanphx
Copy link
Contributor

evanphx commented Feb 2, 2022

@paladin-devops Ok, we're in the home stretch! You'll need to include a changelog entry and then regenerate the website docs by doing: go run ./cmd/waypoint docs -website-mdx

@paladin-devops
Copy link
Contributor Author

@evanphx I pushed my last changes I think right as the GitHub outage happened yesterday! Looking back at this I staged the doc changes for everything, not just the Docker docs. Before merging if you'd like me to undo the changes made to the non-Docker docs, lmk!

@krantzinator
Copy link
Contributor

@paladin-devops It looks like something in your configuration is adding extra spaces when you do the docs generation. Possibly a language server or IDE config? I'm not sure, but if you can't figure it out, I can push up the generated docs if you give me contributor access to your branch.

@paladin-devops
Copy link
Contributor Author

@krantzinator thank you for the fixup! I'll try to figure out what's up with my formatting configs for future doc updates. In this case I was using Atom's go-plus plugin.

@xiaolin-ninja xiaolin-ninja merged commit 197f28e into hashicorp:main Feb 9, 2022
@paladin-devops paladin-devops deleted the feat/docker-builder-auth branch February 25, 2022 18:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Waypoint Docker builder FROM private registry fails
4 participants