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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

enhance: deterministically generate fake arns/ids #2549

Merged
merged 4 commits into from
Jul 6, 2023

Conversation

tim775
Copy link
Member

@tim775 tim775 commented Jul 5, 2023

No description provided.

@tim775 tim775 self-assigned this Jul 5, 2023
@tim775 tim775 requested a review from hugorut July 6, 2023 02:55
@tim775 tim775 marked this pull request as ready for review July 6, 2023 02:55
Copy link
Contributor

@hugorut hugorut left a comment

Choose a reason for hiding this comment

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

Ok so this looks good, and it will work. However I was kinda hoping we could do it another way so that we could get rid of the dynamic count.index and each.value expressions. These are pretty hacky and I don't think work that well. Here's what I was thinking:

/// SetAttributesFunc defines a function that sets required attributes on a hcl.Block.
// This is done so that identifiers that are normally propagated from a Terraform state/apply
// are set on the Block. This means they can be used properly in references and outputs.
type SetAttributesFunc func(block *Block, moduleBlock *Block)

// SetUUIDAttributes adds commonly used identifiers to the block so that it can be referenced by other
// blocks in context evaluation. The identifiers are only set if they don't already exist as attributes
// on the block.
func SetUUIDAttributes(block *Block, moduleBlock *Block) {
	hclBlock := block.hclBlock
	if body, ok := hclBlock.Body.(*hclsyntax.Body); ok {
		if (hclBlock.Type == "resource" || hclBlock.Type == "data") && body.Attributes != nil {
			if _, ok := body.Attributes["id"]; !ok {
				body.Attributes["id"] = newUniqueAttribute("id", block)
			}
                        ...
		}
	}
}

type BlockAddressExpression struct {
	// embedding the LiteralValueExpr is just a hack so that we can conform to the hclsyntax.Expression interface
	// which contains an unexported method walkChildNodes.
	*hclsyntax.LiteralValueExpr

	Block            *Block
	ExpressionPrefix *string
}

func NewBlockAddressExpression(block *Block, prefix *string) *BlockAddressExpression {
	return &BlockAddressExpression{
		LiteralValueExpr: &hclsyntax.LiteralValueExpr{},
		Block:            block,
		ExpressionPrefix: prefix,
	}
}

func (b *BlockAddressExpression) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnostics) {
	val := b.Block.FullName()
	h := sha256.New()
	h.Write([]byte(val))
	val = hex.EncodeToString(h.Sum(nil))
	
	if b.ExpressionPrefix != nil {
		val = fmt.Sprintf("%s%s", *b.ExpressionPrefix, val)
	}

	return cty.StringVal(val), nil
}

func newUniqueAttribute(name string, block *Block) *hclsyntax.Attribute {
	return &hclsyntax.Attribute{
		Name: name,
		Expr: NewBlockAddressExpression(block, nil),
	}
}

func newArnAttribute(name string, block *Block) *hclsyntax.Attribute {
	// fakeARN replicates an aws arn string it deliberately leaves the
	// region section (in between the 3rd and 4th semicolon) blank as
	// Infracost will try and parse this region later down the line.
	// Keeping it blank will defer the region to what the provider has defined.
	fakeARN := "arn:aws:hcl::"
	return &hclsyntax.Attribute{
		Name: name,
		Expr: NewBlockAddressExpression(block, &fakeARN),
	}
}

This way every time the expression is evaluated we get the full block address. This should include things like the count e.g. aws_instance.test[0] and for each, e.g. aws_instance.test['foo'] so we don't need to append the dynamic value on the end.

Plus it simplifies the code a bit and means we don't have to keep track of the withCount and withEach, that I've always hated.

Let me know what you think.

Note: the above is untested, so might actually not work... 馃槵

Copy link
Contributor

@hugorut hugorut left a comment

Choose a reason for hiding this comment

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

Talked over zoom, and this method is preferable

@tim775 tim775 merged commit 37d8f6f into master Jul 6, 2023
9 of 10 checks passed
@tim775 tim775 deleted the enhance/deterministic_fake_arns branch July 6, 2023 20:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants