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

Fix empty "rendered" with FCC 1.1 and snippets #72

Merged
merged 3 commits into from
Nov 20, 2020

Conversation

bendrucker
Copy link
Contributor

@bendrucker bendrucker commented Nov 17, 2020

When pretty_print = false is used along with FCC 1.1 configuration and snippets, the result is an empty (default) Ignition document. Adapting a case from the tests:

terraform {
  required_version = ">= 0.13"

  required_providers {
    ct = {
      source  = "poseidon/ct"
      version = "~> 0.7.0"
    }
  }
}

data "ct_config" "fedora-coreos-snippets" {
  pretty_print = false
  strict = true
  content = <<EOT
---
variant: fcos
version: 1.1.0
passwd:
  users:
    - name: core
      ssh_authorized_keys:
        - key
EOT
	snippets = [
<<EOT
---
variant: fcos
version: 1.1.0
systemd:
  units:
    - name: docker.service
      enabled: true
EOT
	]
}

output "ignition" {
  value = data.ct_config.fedora-coreos-snippets.rendered
}

The result is:

$ terraform apply -auto-approve
data.ct_config.fedora-coreos-snippets: Refreshing state... [id=475385134]

Apply complete! Resources: 0 added, 0 changed, 0 destroyed.

Outputs:

ignition = {"ignition":{"config":{"replace":{"verification":{}}},"proxy":{},"security":{"tls":{}},"timeouts":{}},"passwd":{},"storage":{},"systemd":{}}

This is a regression from 0.6, which outputs the expected rendered result:

$ terraform apply -auto-approve
data.ct_config.fedora-coreos-snippets: Refreshing state... [id=475385134]

Apply complete! Resources: 0 added, 0 changed, 0 destroyed.

Outputs:

ignition = {"ignition":{"config":{"replace":{"verification":{}}},"proxy":{},"security":{"tls":{}},"timeouts":{},"version":"3.1.0"},"passwd":{"users":[{"name":"core","sshAuthorizedKeys":["key"]}]},"storage":{},"systemd":{"units":[{"enabled":true,"name":"docker.service"}]}}

I added test cases where pretty_print is set to false, which demonstrated that this is only an issue with FCC version 1.1. The cause turns out to be the wrong Ignition object being passed to json.MarshalIndent, specifically ign instead of ign31. Passing that in to match json.Marshal results in a passing test.

@bendrucker bendrucker changed the title Fix empty rendered" Fix empty "rendered" with FCC 1.1 and snippets Nov 17, 2020
@bendrucker
Copy link
Contributor Author

I've also pulled out a marshalJSON func that handles switching between Marshal and MarshalIndent to try to prevent similar bugs

@@ -191,6 +191,31 @@ const containerLinuxSnippetsExpected = `{
}
}`

const containerLinuxSnippetsPrettyFalseResource = `
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Instead of creating entirely separate test configs, I could also convert containerLinuxSnippetsResource et al into functions that take a pretty argument and use fmt.Sprintf to render the configuration. Wanted to avoid modifying other test cases initially but happy to do that now if you'd prefer.

Copy link
Member

@dghubble dghubble Nov 20, 2020

Choose a reason for hiding this comment

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

I think its fine. Eventually all this custom merging should be handled upstream in fcct, which would avoid all the branches motivating this exhaustive testing. coreos/butane#120

@dghubble dghubble merged commit b5c39b2 into poseidon:master Nov 20, 2020
@dghubble
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants