Skip to content
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

Add feature to define individual oauth scopes to node pools #94

Conversation

lantier
Copy link
Contributor

@lantier lantier commented Mar 8, 2019

Recently I had a problem where some applications that use specific GCP oauth_scopes didn't recognize the scope cloud-platform (which was the default implementation of this module) as the necessary scope.

Another point is when in some situation we may need to limit scopes for some specific pools or cluster so I was thinking that may be useful to introduce manual implementation of scopes as a feature. (The default still is the cloud-platform

The module execution is similar (as in the README) as the other node_pool configuration:

node_pools_oauth_scopes = {
    all   = ["https://www.googleapis.com/auth/storage-rw"]
    small = [
      "https://www.googleapis.com/auth/cloud-vision",
      "https://www.googleapis.com/auth/ediscovery"
    ]
  }

Copy link
Contributor

@adrienthebo adrienthebo left a comment

Choose a reason for hiding this comment

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

Thanks for submitting this pull request! (And thanks for your patience on getting this review, we've been getting CI issues resolved so that we can validate pull requests like this.)

Before we dig into this change closely, I noticed that oauth scopes on node pools are no longer granted on GKE 1.10 and later. Is this pull request adding support for GKE 1.9 and older, or is there utility for oauth scopes in GKE 1.10+ that will be of continued use?

variables.tf Show resolved Hide resolved
morgante
morgante previously approved these changes Mar 15, 2019
@morgante
Copy link
Contributor

Changes look good, but CI is failing.

@adrienthebo
Copy link
Contributor

Here's a copy of the current CI failures:

-----> Converging <node-pool-local>...
       Terraform v0.11.10
       
       Your version of Terraform is out of date! The latest version
       is 0.11.13. You can update by downloading from www.terraform.io/downloads.html
$$$$$$ Running command `terraform workspace select kitchen-terraform-node-pool-local` in directory /tmp/build/859ab044/terraform-google-kubernetes-engine/test/fixtures/node_pool
$$$$$$ Running command `terraform get -update` in directory /tmp/build/859ab044/terraform-google-kubernetes-engine/test/fixtures/node_pool
       - module.example
         Updating source "../../../examples/node_pool"
       - module.example.gke
         Updating source "../../"
$$$$$$ Running command `terraform validate -check-variables=true   ` in directory /tmp/build/859ab044/terraform-google-kubernetes-engine/test/fixtures/node_pool
       
       Error: module.example.module.gke.google_container_node_pool.zonal_pools: 2 error(s) occurred:
       
       * module.example.module.gke.google_container_node_pool.zonal_pools[0]: key "pool-01" does not exist in map var.node_pools_oauth_scopes in:
       
       ${concat(var.node_pools_oauth_scopes["all"], var.node_pools_oauth_scopes[lookup(var.node_pools[count.index], "name")])}
       * module.example.module.gke.google_container_node_pool.zonal_pools[1]: key "pool-02" does not exist in map var.node_pools_oauth_scopes in:
       
       ${concat(var.node_pools_oauth_scopes["all"], var.node_pools_oauth_scopes[lookup(var.node_pools[count.index], "name")])}
       

I bet that we can resolve this issue by adding a default value for the node_poollookup(var.node_pools[count.index], "name") expression. Perhaps lookup(var.node_pools[count.index], "name", list()) with a default value work work. Could you give that a shot?

@thiagonache
Copy link

Hey folks (@adrienthebo @aaron-lane @Jberlinsky @morgante ), when can we expect it to be merged? This is a very important feature.

@morgante morgante changed the base branch from master to aaron-lane-v2.0.0 April 12, 2019 20:39
@morgante
Copy link
Contributor

This LGTM, I'm going to merge into our v2.0.0 release branch.

@morgante morgante merged commit 439a1e3 into terraform-google-modules:aaron-lane-v2.0.0 Apr 12, 2019
aaron-lane added a commit that referenced this pull request Apr 12, 2019
CPL-markus pushed a commit to WALTER-GROUP/terraform-google-kubernetes-engine that referenced this pull request Jul 15, 2024
…ode-pools-oauth-scopes

Add feature to define individual oauth scopes to node pools
CPL-markus pushed a commit to WALTER-GROUP/terraform-google-kubernetes-engine that referenced this pull request Jul 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants