-
Notifications
You must be signed in to change notification settings - Fork 21
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
fix: allow building aws images using source_ami #723
Conversation
@@ -316,14 +316,15 @@ variable "run_tags" { | |||
# https://www.packer.io/docs/datasources/amazon/ami | |||
data "amazon-ami" "autogenerated_1" { | |||
access_key = var.aws_access_key | |||
filters = { | |||
filters = var.source_ami != "" ? {} : { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested locally passing source AMI
➜ konvoy-image-builder git:(shalin/fix-ami-source) ✗ ./bin/konvoy-image-wrapper build aws --overrides overrides/fips.yaml images/ami/rhel-84-fips.yaml --source-ami ami-07da62bb37a54c3b7
2023/03/09 21:03:57 writing new packer configuration to work/rhel-8.4-fips-1678395837-fZfQj
2023/03/09 21:03:57 starting packer build
amazon-ebs.kib_image: output will be in this color.
==> amazon-ebs.kib_image: Prevalidating any provided VPC information
==> amazon-ebs.kib_image: Prevalidating AMI Name: konvoy-ami-rhel-8.4-fips-1.25.4-fips.0-20230309210405
amazon-ebs.kib_image: Found Image ID: ami-07da62bb37a54c3b7
...
Co-authored-by: Daniel Lipovetsky <3445370+dlipovetsky@users.noreply.github.com>
What problem does this PR solve?:
Packer fails building AMI when user provides
source_ami
instead ofsource_ami_filters
Packer fails when evaluating empty ami_filter string:
""
error:
related upstream issue: hashicorp/packer#11037
Fix:
{}
when source_ami is provided.owner
field. so setting it toself
in case user does not provide it or explicitly set it to empty (""
). see https://developer.hashicorp.com/packer/plugins/builders/amazon/ebs#ownersThis is a special usecase that our e2e tests does not cover.
I verified the fix by testing manually.
Which issue(s) does this PR fix?:
*https://d2iq.atlassian.net/browse/D2IQ-96305
Special notes for your reviewer:
Packer uses HCL2 for its configuration language. This is same language used in Terraform. however, it is more restrictive when used with packer. This makes it difficult to write conditional blocks.
Other options considered:
use
count
to control block execution. This is most preferred way in terraform HCL. This does not work with Packer.error:
An argument named "count" is not expected here
reference: Conditional data sources? hashicorp/packer#11483 (comment)
use dynamic blocks
documentation: https://developer.hashicorp.com/terraform/language/expressions/dynamic-blocks
This solution works at some extent to render empty data resource.
However the
source_ami_filter
block requiresowner
field which makes it difficult to completely omit thedata
resource forami
.use local variables
Packer does not support using local variables in
data
resource block. makes it difficult to create conditional variables.Packer issue: Data Sources cannot use local values hashicorp/packer#11011
Use Go template
9b53818
We have used go template before. While this is a great solution, we will end up with mix of go templates and raw packer files. Raw packer files has advantage of testing them directly without getting rendered by KIB. which makes it easy to iterate when triaging bugs.