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

Feat: add the option to add a regex filter for list of teams #984

Merged
merged 3 commits into from
Jan 14, 2025

Conversation

TomerHeber
Copy link
Collaborator

Issue & Steps to Reproduce / Feature Request

resolves #982

Solution

  1. Added a regex filter to the data_teams schema, and implemented the code.
  2. Added additional acceptance tests.

@TomerHeber
Copy link
Collaborator Author

/review

@bot-env0 bot-env0 requested a review from a team November 23, 2024 17:53
@liranfarage89
Copy link
Contributor

@TomerHeber Im not sure how exactly it resolves #982
dee-kryvenko suggested a workdaround

data "env0_teams" "all_teams" {}

data "env0_team" "compartment_admins" {
  for_each = toset([
    for group in data.env0_teams.all_teams.names : group
    if contains(local.okta_admin_groups, group)
  ])
  name = each.value
}

The downside is that env0_teams data source trying to get a list of all teams in the org, which is excessive and unnecessary. Additionally, it fails agains another issue described in #981

This PR's implementation filters the full backend response.

@TomerHeber
Copy link
Collaborator Author

TomerHeber commented Dec 11, 2024

@TomerHeber Im not sure how exactly it resolves #982 dee-kryvenko suggested a workdaround

data "env0_teams" "all_teams" {}

data "env0_team" "compartment_admins" {
  for_each = toset([
    for group in data.env0_teams.all_teams.names : group
    if contains(local.okta_admin_groups, group)
  ])
  name = each.value
}

The downside is that env0_teams data source trying to get a list of all teams in the org, which is excessive and unnecessary. Additionally, it fails agains another issue described in #981

This PR's implementation filters the full backend response.

Hi @liranfarage89 :

Have you observed the discussion in:
#982

In addition we are now caching the API call. The full teams list will be stored in memory (for every terraform execution).

@liranfarage89
Copy link
Contributor

@TomerHeber I read it and still don't get why this filter is necessary as long as the user suggested a workaround that basically does the same

@TomerHeber
Copy link
Collaborator Author

TomerHeber commented Dec 12, 2024

@TomerHeber I read it and still don't get why this filter is necessary as long as the user suggested a workaround that basically does the same

@liranfarage89

Got it!
Are you okay with closing this issue?

Copy link
Contributor

@liranfarage89 liranfarage89 left a comment

Choose a reason for hiding this comment

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

👍

Writing here for future reference:
Although the filter happens in the client, the teams are globally cached.

@github-actions github-actions bot added ready to merge PR approved - can be merged once the PR owner is ready and removed pending final review labels Jan 14, 2025
@TomerHeber TomerHeber merged commit e89e355 into main Jan 14, 2025
5 checks passed
@TomerHeber TomerHeber deleted the feat-add-filter-for-teams-#982 branch January 14, 2025 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature provider ready to merge PR approved - can be merged once the PR owner is ready
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Need to be able to lookup teams based on the list of names considering that not all of them may exists yet
2 participants