Skip to content

Enable usages of application default credentials #610

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 29, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 11 additions & 6 deletions deploy/kubernetes/deploy-driver.sh
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@
# which are in Kubernetes version 1.10.5+

# Args:
# GCE_PD_SA_DIR: Directory the service account key has been saved in (generated by setup-project.sh)
# GCE_PD_SA_DIR: Directory the service account key has been saved in (generated
# by setup-project.sh). Ignored if GCE_PD_DRIVER_VERSION == noauth.
# GCE_PD_DRIVER_VERSION: The kustomize overlay (located in
# deploy/kubernetes/overlays) to deploy. Can be one of {stable, dev}

Expand Down Expand Up @@ -43,7 +44,9 @@ while [ -n "${1-}" ]; do
esac
done

ensure_var GCE_PD_SA_DIR
if [ "${DEPLOY_VERSION}" != noauth ]; then
ensure_var GCE_PD_SA_DIR
fi

function check_service_account()
{
Expand Down Expand Up @@ -71,7 +74,7 @@ function check_service_account()

ensure_kustomize

if [ "$skip_sa_check" != true ]; then
if [ "$skip_sa_check" != true -a "${DEPLOY_VERSION}" != noauth ]; then
check_service_account
fi

Expand All @@ -80,9 +83,11 @@ then
${KUBECTL} create namespace "${NAMESPACE}" -v="${VERBOSITY}"
fi

if ! ${KUBECTL} get secret cloud-sa -v="${VERBOSITY}" -n "${NAMESPACE}";
then
${KUBECTL} create secret generic cloud-sa -v="${VERBOSITY}" --from-file="${GCE_PD_SA_DIR}/cloud-sa.json" -n "${NAMESPACE}"
if [ "${DEPLOY_VERSION}" != noauth ]; then
if ! ${KUBECTL} get secret cloud-sa -v="${VERBOSITY}" -n "${NAMESPACE}";
then
${KUBECTL} create secret generic cloud-sa -v="${VERBOSITY}" --from-file="${GCE_PD_SA_DIR}/cloud-sa.json" -n "${NAMESPACE}"
fi
fi

# GKE Required Setup
Expand Down
8 changes: 0 additions & 8 deletions deploy/kubernetes/images/dev/image.yaml

This file was deleted.

5 changes: 0 additions & 5 deletions deploy/kubernetes/images/dev/kustomization.yaml

This file was deleted.

13 changes: 7 additions & 6 deletions deploy/kubernetes/overlays/dev/kustomization.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,12 @@ apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
resources:
- ../alpha
patches:
patchesStrategicMerge:
- controller_always_pull.yaml
- node_always_pull.yaml
namespace:
gce-pd-csi-driver
transformers:
- ../../images/dev

namespace: gce-pd-csi-driver
# To change the dev image, add something like the following.
#images:
#- name: gke.gcr.io/gcp-compute-persistent-disk-csi-driver
# newName: gcr.io/mattcary-gke-dev-owned/csi/gce-pd-driver
# newTag: latest.
Copy link
Contributor

Choose a reason for hiding this comment

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

why you prefer to change image here instead of under /image dir?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@verult

I just figured this out this morning, it's a little complicated.... The actual change is to remove the change to the dev driver image, as currently the image in images/dev/images.yaml is not the same as the placeholder in base/.

I add a new overlay noauth which removes the secret volume. It uses the dev overlay as a resource. If the dev overlay uses the image resource, as far as I can tell it overrides the image before any images: transformer in the noauth overlay.

Because the images transform name: must match what's already in the yaml, that would mean for any images: transform (in overlays/noauth/kustomize.yaml) would have to match whatever dev is transformed into, that is, the newName in /images/dev/images.yaml (which is currently David's repo ;-)

When the pd csi driver k8s-integration test runs with --do-driver-build=true, the image tag to load is set by doing kustomize edit in the overlay dir, eg overlays/dev or overlays/noauth. That edit assumes the name is the placeholder (gke.gcr.io/gcp-compute-persiistent-disk-csi-driver). But, if the dev/ overlay has already changed the image name, then that kustomize edit isn't effective (since name: doesn't match anything).

Possibly one could fix this by doing a strategicMergePatch that matches the driver container rather than using an images transform? Then we wouldn't have this problem of import dependancies.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think k8s-integration could potentially create its own image tag instead of overwriting the file. Do you think that would solve the problem?

The other way could be to move the noauth patch up one level and include it as part of the dev overlay itself.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

k8s-integration would have to know which images/ transform to change.

Moving the noauth patch up would help, I wasn't sure if we wanted to keep a use-key dev for those who aren't restricted about key downloads (because the extra cluster create parameters could break some workflows maybe)

I also looked into sharing the always-push change although the easiest way to do that means passing the no load restriction flag.

7 changes: 7 additions & 0 deletions deploy/kubernetes/overlays/noauth/kustomization.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
resources:
- ../dev
patchesStrategicMerge:
- noauth.yaml
namespace: gce-pd-csi-driver
25 changes: 25 additions & 0 deletions deploy/kubernetes/overlays/noauth/noauth.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
kind: Deployment
apiVersion: apps/v1
metadata:
name: csi-gce-pd-controller
spec:
template:
spec:
containers:
- name: gce-pd-driver
env:
- $patch: delete
name: GOOGLE_APPLICATION_CREDENTIALS
value: "/etc/cloud-sa/cloud-sa.json"
volumeMounts:
- $patch: delete
name: cloud-sa-volume
readOnly: true
mountPath: "/etc/cloud-sa"
volumes:
- $patch: delete
name: cloud-sa-volume
secret:
secretName: cloud-sa


32 changes: 18 additions & 14 deletions test/k8s-integration/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,25 +56,29 @@ func installDriver(platform, goPath, pkgDir, stagingImage, stagingVersion, deplo
}
}

// setup service account file for secret creation
tmpSaFile := filepath.Join(generateUniqueTmpDir(), "cloud-sa.json")
defer removeDir(filepath.Dir(tmpSaFile))
var deployEnv []string
if deployOverlayName != "noauth" {
// setup service account file for secret creation
tmpSaFile := filepath.Join(generateUniqueTmpDir(), "cloud-sa.json")
defer removeDir(filepath.Dir(tmpSaFile))

// Need to copy it to name the file "cloud-sa.json"
out, err := exec.Command("cp", *saFile, tmpSaFile).CombinedOutput()
if err != nil {
return fmt.Errorf("error copying service account key: %s, err: %v", out, err)
}
defer shredFile(tmpSaFile)

// Need to copy it to name the file "cloud-sa.json"
out, err := exec.Command("cp", *saFile, tmpSaFile).CombinedOutput()
if err != nil {
return fmt.Errorf("error copying service account key: %s, err: %v", out, err)
deployEnv = append(deployEnv, fmt.Sprintf("GCE_PD_SA_DIR=%s", filepath.Dir(tmpSaFile)))
}
defer shredFile(tmpSaFile)

// deploy driver
deployCmd := exec.Command(filepath.Join(pkgDir, "deploy", "kubernetes", "deploy-driver.sh"), "--skip-sa-check")
deployCmd.Env = append(os.Environ(),
deployEnv = append(deployEnv,
fmt.Sprintf("GOPATH=%s", goPath),
fmt.Sprintf("GCE_PD_SA_DIR=%s", filepath.Dir(tmpSaFile)),
fmt.Sprintf("GCE_PD_DRIVER_VERSION=%s", deployOverlayName),
)
err = runCommand("Deploying driver", deployCmd)
fmt.Sprintf("GCE_PD_DRIVER_VERSION=%s", deployOverlayName))
deployCmd.Env = append(os.Environ(), deployEnv...)
err := runCommand("Deploying driver", deployCmd)
if err != nil {
return fmt.Errorf("failed to deploy driver: %v", err)
}
Expand All @@ -87,7 +91,7 @@ func installDriver(platform, goPath, pkgDir, stagingImage, stagingVersion, deplo
klog.Infof("Waiting 5 minutes for the driver to start on Linux")
time.Sleep(5 * time.Minute)
}
out, err = exec.Command("kubectl", "describe", "pods", "-n", driverNamespace).CombinedOutput()
out, err := exec.Command("kubectl", "describe", "pods", "-n", driverNamespace).CombinedOutput()
klog.Infof("describe pods \n %s", string(out))

if err != nil {
Expand Down
4 changes: 3 additions & 1 deletion test/k8s-integration/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,9 @@ func main() {
ensureVariable(deployOverlayName, false, "'deploy-overlay-name' must not be set when using GKE managed driver")
}

ensureVariable(saFile, true, "service-account-file is a required flag")
if *deployOverlayName != "noauth" {
ensureVariable(saFile, true, "service-account-file is a required flag")
}
if !*useGKEManagedDriver {
ensureVariable(deployOverlayName, true, "deploy-overlay-name is a required flag")
}
Expand Down
32 changes: 24 additions & 8 deletions test/run-k8s-integration-local.sh
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ readonly test_version=${TEST_VERSION:-master}

source "${PKGDIR}/deploy/common.sh"

ensure_var GCE_PD_SA_DIR

make -C "${PKGDIR}" test-k8s-integration

# This version of the command creates a GKE cluster. It also downloads and builds a k8s release
Expand Down Expand Up @@ -73,11 +71,29 @@ make -C "${PKGDIR}" test-k8s-integration
# --gce-zone="us-central1-c" --num-nodes=${NUM_NODES:-3} --gke-release-channel="rapid" --deployment-strategy="gke" \
# --use-gke-managed-driver=true --teardown-cluster=true

# This version of the command does not build the driver or K8s, points to a
# local K8s repo to get the e2e.test binary, and does not bring up or down the cluster

# This version of the command does not build the driver or K8s, points to a local K8s repo to get
# the e2e.test binary, does not bring up or down the cluster, and uses application default
# credentials instead of requiring a service account key.
#
# Cluster nodes must have the proper GCP scopes set. This is done with kubetest by
# NODE_SCOPES=https://www.googleapis.com/auth/cloud-platform \
# KUBE_GCE_NODE_SERVICE_ACCOUNT=$SERVICE_ACCOUNT_NAME@$PROJECT.iam.gserviceaccount.com \
# kubetest --up
#
# GCE_PD_SA_DIR is not used.
#
# As with all other methods local credentials must be set by running
# gcloud auth application-default login
"${PKGDIR}/bin/k8s-integration-test" --run-in-prow=false \
--staging-image="${GCE_PD_CSI_STAGING_IMAGE}" --service-account-file="${GCE_PD_SA_DIR}/cloud-sa.json" \
--deploy-overlay-name=dev --bringup-cluster=false --teardown-cluster=false --local-k8s-dir="$KTOP" \
--storageclass-files=sc-standard.yaml,sc-balanced.yaml,sc-ssd.yaml --do-driver-build=false --test-focus='External.Storage' \
--deploy-overlay-name=noauth --bringup-cluster=false --teardown-cluster=false --local-k8s-dir="$KTOP" \
--storageclass-files=sc-standard.yaml --do-driver-build=false --test-focus='External.Storage' \
--gce-zone="us-central1-b" --num-nodes="${NUM_NODES:-3}"


# This version of the command does not build the driver or K8s, points to a
# local K8s repo to get the e2e.test binary, and does not bring up or down the cluster
# "${PKGDIR}/bin/k8s-integration-test" --run-in-prow=false \
# --staging-image="${GCE_PD_CSI_STAGING_IMAGE}" --service-account-file="${GCE_PD_SA_DIR}/cloud-sa.json" \
# --deploy-overlay-name=dev --bringup-cluster=false --teardown-cluster=false --local-k8s-dir="$KTOP" \
# --storageclass-files=sc-standard.yaml --do-driver-build=false --test-focus='External.Storage' \
# --gce-zone="us-central1-b" --num-nodes="${NUM_NODES:-3}"