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

[Feature] slim down to null_data_source and exclude_label_tags #23

Closed
wants to merge 1 commit into from
Closed

Conversation

dennybaa
Copy link

Motivation is to provide an abstract base for other null_labels.

/* some tf file */
module "project" {
  ...
  user_tags_only = true
  tags = {
    ManagedBy = "terraform"
    Foo = "bar"
  }
}

/* using an abstract base to initialize with inherited tags */
module "kops" {
   name = "kops"
   namespace = "${module.project.namespace}"
   stage = "${module.project.stage}"
   tags = "${module.project.tags}
}

@osterman osterman self-assigned this Jun 26, 2018
@osterman osterman added the enhancement New feature or request label Jun 26, 2018
@osterman
Copy link
Member

I think this is a good suggestion. Can we think of another name for user_tags_only?

e.g. override_tags

@osterman
Copy link
Member

@Jamie-BitFlight what do you think about this enhancement?

Also, I think there's some danger here if the Name tag is not overriden as then there's signficicant risk of collisions.

@osterman
Copy link
Member

Also, what about doing something different: adding support for a context output which is a map of the current label state. Then we support an optional context variable. This way we could pass state between modules relatively easily.

/* some tf file */
module "project" {
  ...
  user_tags_only = true
  tags = {
    ManagedBy = "terraform"
    Foo = "bar"
  }
}
/* using an abstract base to initialize with inherited tags */
module "kops" {
   name = "kops"
   context = "${module.project.context}"
}

outputs.tf Outdated
@@ -24,7 +24,7 @@ output "attributes" {
}

output "tags" {
value = "${local.tags}"
value = "${local.tags[var.user_tags_only ? "user" : "all"]}"
description = "Normalized Tag map"
Copy link
Author

@dennybaa dennybaa Jun 27, 2018

Choose a reason for hiding this comment

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

Also, you've mentioned tags as normalized . In fact there's no normalization... keys are untouched and basically everything is outputted exactly as it was passed via variable plus Name/Stage/Namespace...

Copy link
Member

Choose a reason for hiding this comment

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

These are the normalized fields:
https://github.com/stackfeed/terraform-null-label/blob/c0d03bea486fc175d6e555737bd98acb4235fb29/main.tf#L3-L7

They are lowercased. Attributes are joined with a delimiter.

Copy link
Author

Choose a reason for hiding this comment

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

yup, right i've missed that

@dennybaa dennybaa changed the title [Feature] slim down to null_data_source and user_tags_only [Feature] slim down to null_data_source and exclude_label_tags Jun 27, 2018
@dennybaa
Copy link
Author

dennybaa commented Jun 27, 2018

@osterman Hey, I've actually fixed the commit. Not sure that override_tags is a good and self-explanatory variable name. This time I come out with a name exclude_label_tags which are Name/Namespace/Stage, it sounds good for me...

@osterman
Copy link
Member

What you do you think about renaming exclude_label_tags to generate_tags or auto_generate_tags? So if generate_tags is false, we do not add the Name, Stage, and Namespace tags.

@osterman
Copy link
Member

@dennybaa checkout this alternative: #25

@aknysh
Copy link
Member

aknysh commented Aug 24, 2018

Addressed in #36

@aknysh aknysh closed this Aug 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants