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 teams common variable #239

Merged
merged 7 commits into from
Feb 22, 2021
Merged

add teams common variable #239

merged 7 commits into from
Feb 22, 2021

Conversation

frederic-peraud
Copy link
Contributor

This PR adds Teams as a common parameter. It allows to link detector(s) to Sfx Teams.

https://registry.terraform.io/providers/splunk-terraform/signalfx/latest/docs/resources/detector#teams

@frederic-peraud frederic-peraud added detectors About nex or existing detectors enhancement Enhancement of existing (templating or detectors) labels Jan 14, 2021
@frederic-peraud frederic-peraud self-assigned this Jan 14, 2021
@frederic-peraud frederic-peraud linked an issue Jan 14, 2021 that may be closed by this pull request
@frederic-peraud frederic-peraud changed the title Common : add teams common parameter DRAFT : Common : add teams common parameter Jan 14, 2021
@xp-1000 xp-1000 marked this pull request as draft January 14, 2021 15:30
@xp-1000 xp-1000 changed the title DRAFT : Common : add teams common parameter add teams common variable Jan 14, 2021
@xp-1000
Copy link
Contributor

xp-1000 commented Jan 14, 2021

thanks @frederic-peraud

I pushed atomic commits to detail all steps to make this kind of change:

  • add the variable declaration, this is what you already did. it will be obviously more painful if it should be declared at detector level.
  • manually edit all "handmade" detectors tf files (no -gen in names) to use the variable. I use a crappy bash command for that: find -type f -name 'detectors-*.tf' ! -name 'detectors-gen.tf' -exec sed -E -i -r 's/^( *name *= format.*)$/\1\n\n toto = var.toto/' {} \; it will add toto definition to all detectors (not generated). aae48b4 (#239)
  • then you will have to update the jinj2 template generator to add this definition, it is generally easy except that we have to be careful on keep a rendered output terraform fmt-ed in all scenarios (depending on yaml conf). 909b442 (#239)
  • Finally run make update-module to let the generator do the job for the rest of files: 83af1ad (#239)

the value assigned to teams attribute and the last commit propose to :

  • take the authorized_writer_teams value if it is defined but not teams
  • take the teams value only if defined (not matter if authorized_writer_teams is defined or not).

It is only a proposition, if you disagree simply reset the last commit and update the definition of teams in all detectors (and the generator) to simply use the teams variable (remove try, coalescelist and authorized_writer_teams)

for now, nobody uses existing authorized_writer_teams which could also interest you and obviously you will be the first user of teams so please challenge this proposition to try to propose something which could help the user but keeping safe and with sens.

when it is ok for you please "publish" the draft PR to warn code owners they can review/approve this.

@frederic-peraud
Copy link
Contributor Author

Hi @xp-1000

Your suggestions are ok for me and my team.

@frederic-peraud frederic-peraud marked this pull request as ready for review January 18, 2021 09:35
@xp-1000 xp-1000 added this to In progress in Claranet via automation Jan 25, 2021
@xp-1000 xp-1000 added the templating About modules templating or terraform capabilities label Jan 25, 2021
@xp-1000 xp-1000 added this to the v1.2.0 milestone Jan 25, 2021
Claranet automation moved this from In progress to Reviewer approved Feb 22, 2021
@xp-1000 xp-1000 merged commit 124fa2e into master Feb 22, 2021
Claranet automation moved this from Reviewer approved to Done Feb 22, 2021
@xp-1000 xp-1000 deleted the add_teams_global_parameter branch February 22, 2021 18:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
detectors About nex or existing detectors enhancement Enhancement of existing (templating or detectors) templating About modules templating or terraform capabilities
Projects
Development

Successfully merging this pull request may close these issues.

Add Teams Global parameter
3 participants