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

google_access_context_manager_access_level_condition should provide an option to add the new condition to the existing condition with an "OR" operator #8125

Open
anubbhavm opened this issue Jan 6, 2021 · 4 comments

Comments

@anubbhavm
Copy link

anubbhavm commented Jan 6, 2021

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment. If the issue is assigned to the "modular-magician" user, it is either in the process of being autogenerated, or is planned to be autogenerated soon. If the issue is assigned to a user, that user is claiming responsibility for the issue. If the issue is assigned to "hashibot", a community member has claimed the issue already.

Description

Introduce a new parameter "operator" to "google_access_context_manager_access_level_condition" to provide an option to add the new condition to the existing condition with an "OR" operator. Currently, the new condition gets added with an "AND" operator. See below for example.

Include all the following attributes
Regions match CH (Switzerland), IT (Italy), and US (United States)
Device screen lock
OS restrictions match Chrome OS (any version)
Members match [email protected]
AND
Include all the following attributes
IP Subnetworks match 192.0.4.0/24
Regions match IT (Italy) and US (United States)
Device corp owned
OS restrictions match Chrome OS (any version)
Members match [email protected], [email protected], and [email protected]

New or Affected Resource(s)

  • google_access_context_manager_access_level_condition

Potential Terraform Configuration

resource "google_access_context_manager_access_level" "access-level-service-account" {
  parent = "accessPolicies/410809943175"
  name   = "accessPolicies/410809943175/accessLevels/tf_test_new_resource"
  title  = "tf_test_new_resource"
  basic {
    conditions {
      device_policy {
        require_screen_lock = true
        os_constraints {
          os_type = "DESKTOP_CHROME_OS"
        }
      }
      members = ["serviceAccount:[email protected]"]
      regions = [
  "CH",
  "IT",
  "US",
      ]
    }
  }

  lifecycle {
    ignore_changes = [basic.0.conditions]
  }
}

resource "google_service_account" "created-later" {
  account_id = "tf-test-123"
}

resource "google_access_context_manager_access_level_condition" "access-level-conditions" {
  access_level = google_access_context_manager_access_level.access-level-service-account.name
  operator = "OR"
  ip_subnetworks = ["192.0.4.0/24"]
  members = ["user:[email protected]", "user:[email protected]", "serviceAccount:${google_service_account.created-later.email}"]
  negate = false
  device_policy {
    require_screen_lock = false
    require_admin_approval = false
    require_corp_owned = true
    os_constraints {
      os_type = "DESKTOP_CHROME_OS"
    }
  }
  regions = [
    "IT",
    "US",
  ]
}

b/359383500

@ghost ghost added enhancement labels Jan 6, 2021
@rileykarson rileykarson assigned melinath and unassigned melinath Jan 19, 2021
@rileykarson rileykarson added this to the Goals milestone Jan 25, 2021
modular-magician added a commit to modular-magician/terraform-provider-google that referenced this issue Jun 12, 2023
modular-magician added a commit that referenced this issue Jun 12, 2023
@github-actions github-actions bot added service/accesscontextmanager forward/review In review; remove label to forward labels Aug 17, 2023
@ggtisc ggtisc self-assigned this Aug 13, 2024
@ggtisc
Copy link
Collaborator

ggtisc commented Aug 13, 2024

The user is looking too add a new argument called operator to manage different operators like OR to the google_access_context_manager_access_level_condition resource.

@ggtisc ggtisc removed their assignment Aug 13, 2024
@ggtisc ggtisc removed the forward/review In review; remove label to forward label Aug 13, 2024
@roaks3
Copy link
Collaborator

roaks3 commented Sep 24, 2024

Note that this should technically be possible through the API by using combiningFunction, but I believe it should be a change to google_access_context_manager_access_level. Adding it to the condition presents 2 problems: it becomes unclear which resource controls the behavior, and the combiningFunction field exists one level up from the fields that the condition resource interacts with.

@coder-221
Copy link

I believe this is already possible with the combining_function property on the google_access_context_manager_access_level resource. For example:

resource "google_access_context_manager_access_level" "access-level-service-account" {
  parent = "accessPolicies/123"
  name   = "accessPolicies/123/accessLevels/tf_test"
  title  = "tf_test_new_resource"
  basic {
    combining_function = "OR" # <--- can be "OR" or "AND"
    conditions {
      regions = [
        "AL",
      ]
    }
  }

  lifecycle {
    ignore_changes = [basic.0.conditions]
  }
}

This matches the API schema. Adding a combining_function to the google_access_context_manager_access_level_condition would diverge from the API and product experience. All of the conditions must be combined with OR or AND (ie some can't be OR and others AND). So I believe keeping this on the level is correct.

I think this issue can be closed but feel free to ask any followup questions.

@Charlesleonius
Copy link

@rileykarson @roaks3 Can we please close this. As @coder-221 explained, this is the expected behavior and design of the resources.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants