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

No obvious support for setting headers for inline service checks #542

Closed
chrisjohnson opened this issue Jul 21, 2020 · 9 comments
Closed

Comments

@chrisjohnson
Copy link
Contributor

Code:

...
  ::consul::service { "test":
    port   => 443,
    checks => [
      {
        name => "Heartbeat check for ${::app_id}-${::app_name}.testing.site.com",
        http => "https://127.0.0.1:443/_heartbeat",
        tls_skip_verify => true,
        method => "GET",
        header => { Host => ["${::app_id}-${::app_name}}.testing.site.com"] },
        interval => "1s",
        timeout => "5s"
      }
    ],
  }

Error:

Error while evaluating a Resource Statement, Consul::Service[test]: parameter 'checks' index 0 entry 'header' expects a Data value, got Hash at /etc/puppetlabs/code/environments/deployme_modern_consul/site/profile/mani
fests/test.pp:7

The expected JSON should be:

...
    {
      "name": "test",
      "port": 443,
      "checks": [
        {
          "name": "Heartbeat check for rx3-test.testing.site.com",
          "http": "https://127.0.0.1:443/_heartbeat",
          "tls_skip_verify": true,
          "method": "GET",
          "header": {
            "Host": [
              "rx3-test.testing.site.com"
            ]
          },
          "interval": "1s",
          "timeout": "5s"
        }
      ]
    }
...

We are using an older version of puppet and only using 5.1.0 of the module, however looking at the master branch, it doesn't seem much has changed in this area.

If at all possible, back-porting the fix to the 5.x line would be greatly appreciated :)

@chrisjohnson
Copy link
Contributor Author

It looks like using this form works header => { "Host" => ["${::app_id}-${::app_name}}.testing.site.com"] },

So maybe this ticket should just be, add an example header config? It was tough to figure this out and we basically lucked into it

@solarkennedy
Copy link
Contributor

I think just this issue is an OK way to document this.

It is puppet, so a bare Host in part of a hash... well I don't know what it will do.
We know what in the end this needs to be a string in the file, so quoting it explicitly is a good thing I think.

@chrisjohnson
Copy link
Contributor Author

chrisjohnson commented Jul 22, 2020 via email

@solarkennedy
Copy link
Contributor

I don't think we need this kind of documentation on the README. This is just general puppet knowledge about putting quotes around strings right? https://puppet.com/docs/puppet/6.17/lang_data_hash.html#syntax

Hash keys can be any data type, but generally, you should use only strings. Put quotation marks around keys that are strings.

@chrisjohnson
Copy link
Contributor Author

chrisjohnson commented Jul 23, 2020 via email

@chrisjohnson
Copy link
Contributor Author

chrisjohnson commented Jul 23, 2020

Sorry, what I mean is that this particular field differs from the rest. I don't really see the harm in adding to the documentation for those of us who arent puppet experts. But if you think it's harmful then I guess that's your call

Edit in my example the entire service definition is a hash, or so it appears to my eyes. That the nested sub hash differs from the hash it's nested within is what is confusing

@solarkennedy
Copy link
Contributor

You are right, it isn't obvious which keys are arguments to a module and which keys are actually k/v strings in a hash.
I would accept a PR.

@chrisjohnson
Copy link
Contributor Author

#543 Thank you!

@chrisjohnson
Copy link
Contributor Author

Resolved

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

No branches or pull requests

2 participants