Implement module for CloudWatch logs #1

SweetOps · 2017-11-01T10:27:26Z

What

Implement module for CloudWatch logs

Why

We need that for collection logs from instances

SweetOps · 2017-11-02T15:34:55Z

osterman · 2017-11-03T17:26:52Z

iam_user.tf

@@ -0,0 +1,43 @@
+resource "aws_iam_user" "default" {


Can we use this module instead: https://github.com/cloudposse/terraform-aws-iam-system-user

@osterman, in my case user will be create if username is defined, with module terraform-aws-iam-system-user can't get same functional. The one way is add count to module terraform-aws-iam-system-user.

@osterman , I made PR in cloudposse/terraform-aws-iam-system-user#2 (comment)

osterman · 2017-11-06T20:34:42Z

iam_user.tf

@@ -0,0 +1,33 @@
+data "aws_iam_policy_document" "user" {
+  count = "${var.create_user == "true" ? 1 : 0}"


let's rename to user_enabled

osterman · 2017-11-06T20:35:32Z

variables.tf

+}
+
+variable "stream_names" {
+  default     = ["pfsense", "ubuntu"]


pfsense is a very specific use case. It doesn't make sense as a global default.

let's just create a default stream with the name of default

osterman · 2017-11-09T06:00:09Z

I activated TravisCI. Please bump.

const-bon · 2017-11-13T14:06:56Z

iam.tf

+
+    principals {
+      type        = "Service"
+      identifiers = ["logs.${length(var.region) > 0 ? var.region: data.aws_region.default.name}.amazonaws.com"]


Document var.region in README.

const-bon · 2017-11-13T14:12:31Z

iam.tf

+      "logs:DeleteLogStream",
+    ]
+
+    resources = [


Use compact with concat.

Use compact with concat.

It can break the module work - with json need be very careful

Explain why it can break json

I'm not sure which delimiter between elements which I'll get after the concat()

It seems, there is no need to use join with delimiter here.
https://www.terraform.io/docs/providers/aws/d/iam_policy_document.html

const-bon · 2017-11-13T14:15:07Z

iam.tf

+
+    resources = [
+      "${aws_cloudwatch_log_group.default.arn}",
+      "${join(",", aws_cloudwatch_log_stream.default.*.arn)}",


Is join("," deliberate?

yes, for prevent mistakes in json

explain what mistakes it prevents. it's not clear.

element in resources should be separated by comma, I'm not sure that comma will be without join()

const-bon · 2017-11-13T14:15:45Z

main.tf

@@ -0,0 +1,35 @@
+data "aws_region" "default" {
+  current = true


const-bon · 2017-11-14T15:03:22Z

@SweetOps @osterman
Should we rename module to terraform-aws-cloudwatch-log-stream for consistence?

SweetOps · 2017-11-16T14:32:07Z

@comeanother , I'm prefer the current name of repo :)

const-bon · 2017-11-22T10:42:11Z

variables.tf

+
+variable "namespace" {
+  description = "Namespace (e.g. `cp` or `cloudposse`)"
+  default     = "cp"


Drop default value.

const-bon · 2017-11-22T10:42:14Z

variables.tf

+}
+
+variable "stage" {
+  default     = "dev"


Drop default value.

const-bon · 2017-11-22T10:45:02Z

iam_user.tf

+  }
+}
+
+module "user" {


Do we need this module?
It is not in use.

It is not in use.

Why do you decided that?

I assume, purpose of this user is to be able to connect to the corresponding streams.
But this user would has access to all streams. https://github.com/cloudposse/terraform-aws-cloudwatch-logs/pull/1/files#diff-a83a8bb953f15707ac8beb549b114ab5R22

const-bon · 2017-11-22T10:47:50Z

iam.tf

+      "logs:DeleteLogStream",
+    ]
+
+    resources = [


It seems, there is no need to use join with delimiter here.
https://www.terraform.io/docs/providers/aws/d/iam_policy_document.html

SweetOps · 2017-11-22T10:55:46Z

It seems, there is no need to use join with delimiter here.
https://www.terraform.io/docs/providers/aws/d/iam_policy_document.html

@comeanother , I see in example resources separated by comma

const-bon · 2017-11-24T14:21:33Z

outputs.tf

+  value = "${aws_cloudwatch_log_group.default.name}"
+}
+
+output "log_stream_name" {


log_stream_names.

const-bon · 2017-11-24T14:21:45Z

outputs.tf

+  description = "ARN of the log group"
+}
+
+output "stream_arn" {


stream_arns.

const-bon · 2017-11-24T14:23:50Z

main.tf

+  name       = "${var.name}"
+  stage      = "${var.stage}"
+  delimiter  = "${var.delimiter}"
+  attributes = "${compact(concat(var.attributes, list("log")))}"


Should we add group attribute?

const-bon · 2017-11-24T14:24:49Z

iam_user.tf

+  }
+}
+
+module "user" {


I assume, purpose of this user is to be able to connect to the corresponding streams.
But this user would has access to all streams. https://github.com/cloudposse/terraform-aws-cloudwatch-logs/pull/1/files#diff-a83a8bb953f15707ac8beb549b114ab5R22

init

570086b

SweetOps requested review from osterman and const-bon November 1, 2017 10:27

SweetOps self-assigned this Nov 1, 2017

SweetOps added 2 commits November 1, 2017 12:36

added CI

81accc4

added user cretion

508d6f2

SweetOps added the wip Work in Progress: Not ready for final review or merge label Nov 1, 2017

SweetOps added 3 commits November 2, 2017 16:45

fix typo

943f046

reworked policy for user

05dc4bb

fixed output

ac0a4b5

SweetOps removed the wip Work in Progress: Not ready for final review or merge label Nov 2, 2017

osterman reviewed Nov 3, 2017

View reviewed changes

rework for using the module terraform-aws-iam-system-user

0442e65

osterman reviewed Nov 6, 2017

View reviewed changes

SweetOps added 2 commits November 7, 2017 16:11

small fixes

554b0f3

update output in readme

d04e2ea

SweetOps added 2 commits November 10, 2017 17:57

travis

bda920f

travis #1

0e83f3e

const-bon reviewed Nov 13, 2017

View reviewed changes

convetions =\

ec3f920

compromiss

6f51722

osterman approved these changes Nov 21, 2017

View reviewed changes

const-bon reviewed Nov 22, 2017

View reviewed changes

fix defaults in vars

7558b49

added new outputs

47b6014

const-bon reviewed Nov 24, 2017

View reviewed changes

SweetOps and others added 3 commits November 24, 2017 17:36

typo

d59301a

fix output

22d6c33

Fix IAM user creation

4714df8

const-bon merged commit 2243829 into master Nov 27, 2017

const-bon deleted the add_code branch November 27, 2017 17:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement module for CloudWatch logs #1

Implement module for CloudWatch logs #1

SweetOps commented Nov 1, 2017

SweetOps commented Nov 2, 2017

osterman Nov 3, 2017

SweetOps Nov 6, 2017

SweetOps Nov 6, 2017

osterman Nov 6, 2017

osterman Nov 6, 2017

osterman Nov 6, 2017

osterman commented Nov 9, 2017

const-bon Nov 13, 2017

const-bon Nov 13, 2017

SweetOps Nov 14, 2017

osterman Nov 15, 2017

SweetOps Nov 15, 2017 •

edited

Loading

const-bon Nov 22, 2017

const-bon Nov 13, 2017

SweetOps Nov 14, 2017

osterman Nov 15, 2017

SweetOps Nov 15, 2017

const-bon Nov 13, 2017

const-bon commented Nov 14, 2017

SweetOps commented Nov 16, 2017

const-bon Nov 22, 2017

const-bon Nov 22, 2017

const-bon Nov 22, 2017

SweetOps Nov 22, 2017

const-bon Nov 24, 2017

const-bon Nov 22, 2017

SweetOps commented Nov 22, 2017

const-bon Nov 24, 2017

const-bon Nov 24, 2017

const-bon Nov 24, 2017

const-bon Nov 24, 2017

		@@ -0,0 +1,33 @@
		data "aws_iam_policy_document" "user" {
		count = "${var.create_user == "true" ? 1 : 0}"

Implement module for CloudWatch logs #1

Implement module for CloudWatch logs #1

Conversation

SweetOps commented Nov 1, 2017

What

Why

SweetOps commented Nov 2, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

osterman commented Nov 9, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SweetOps Nov 15, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

const-bon commented Nov 14, 2017

SweetOps commented Nov 16, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SweetOps commented Nov 22, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SweetOps Nov 15, 2017 •

edited

Loading