Add Sentinel HCL support

[glennsarti]

This commit adds a Sentinel HCL specific grammar file. This should be used
with the root Sentinel configuration file (Sentinel.hcl) as per [1]. It
should also be used with Sentinel Test configuration files (*.hcl) as per [2]

[1] https://docs.hashicorp.com/sentinel/configuration#configuration-file-reference
[2] https://docs.hashicorp.com/sentinel/writing/testing#test-case-format

[glennsarti]

poke @radeksimko

[radeksimko]

Thanks for the ping @glennsarti and sorry for the delay!

We're having a team off-site this week, but I'll try to get it reviewed by Friday.

[radeksimko]

Hi @glennsarti
Thanks for the contribution.

The two use cases you mentioned (config file & test case) seem to me like two different languages that would each deserve its own grammar? Generally the reason we create dedicated TM grammars here is to account for the HCL schema (i.e. basically differences in keywords such as module or resource). There's a bit more detail under https://github.com/hashicorp/syntax#hcl-vs-terraform--packer-- but to put it simply - HCL defines the structures like {, }, =, where they are etc. and product uses a schema which then defines the keywords for block types, labels and attributes.

On a related note, the grammar you attached doesn't seem to be adding any particular keywords, it would highlight files the same way as plain HCL from what I can tell?
You can see what we do for the TF grammar here for reference:

syntax/src/terraform.yml

Line 15 in 9d47edc

- match: \bresource|provider|variable|output|locals|module|data|terraform\b

syntax/src/terraform.yml

Line 68 in 9d47edc

match: \b(var|local|module|data|path|terraform)\b

What are you plans with regards to matching the files which you'd highlight with this grammar? For the config file I understand that you'd match exactly sentinel.hcl, but the test cases seem to be using just the "product/schema-agnostic" extension *.hcl. Have you considered establishing a convention such as *.sentineltest.hcl or any kind of unique, shorter version of that? This is a reason why Terraform uses *.tf & *.tfvars or Packer *.pkr.hcl & *.pkrvars.hcl (the latter being more modern convention).

Choosing *.anything.hcl allows you to leverage the existing HCL grammar for basic highlighting - which e.g. Packer users already do without us having to maintain a dedicated Packer grammar.

See also hashicorp/vscode-hcl#116 and hashicorp/nomad#13350 TL;DR the existing HCL grammar can already highlight Nomad job spec, even though those file don't yet follow the modern naming convention. The only downside is that users have to add a piece of config mapping *.nomad to HCL (language).

Let me know if you prefer to discuss this on Zoom or Slack - I'm available there too, although I have some planned annual leave coming up from Wednesday 24th.

[glennsarti]

Hi @radeksimko

The two use cases you mentioned (config file & test case) seem to me like two different languages that would each deserve its own grammar?

They are two different use cases but they use the same language so I don't think they need separate grammars. It is legal (as far as the language is concerned) to use "test" things in a config file and vice versa. Those distinctions belong in static code analysis. A close analogy would be;

You can put Azure things in Terraform Language, or you can put AWS things in the Terraform Language. Both use the TF language, but are different usecases.

Why did I create a new language instead of using the default?

I did list this in the grammar file but it could be a bit hard to see

    # Sentinel attributes can include a forward slash in the name
    begin: ([\w][\-\w]*)([\s\"\-\w\/]*)(\{)

The current HCL language file will not recognise some legitimate Sentinel HCL code.

Looks like I need restrict the block name types though to correspond to the Sentinel language which is fine.

What are you plans with regards to matching the files which you'd highlight with this grammar? For the config file I understand that you'd match exactly sentinel.hcl, but the test cases seem to be using just the "product/schema-agnostic" extension *.hcl

So yeah, the configuration file has a best practice of being named sentinel.hcl but the tests are detected via the policy and directory structure i.e. for a policy called foo, the policy is at ./foo.sentinel and the tests are ./test/foo/*.hcl

But to be fair, that's not the concern of the grammar file though. The editor should be deciding what grammar to use for which file. This filetypes parameter is just a hint to the Textmate editor (not even a vscode hint)

[radeksimko]

I did list this in the grammar file but it could be a bit hard to see
# Sentinel attributes can include a forward slash in the name

I believe you are talking about block labels rather than attributes here?

block_type "label1" "label2" {
  # block body
  attribute_name = expression
}

I'm assuming we're talking about this example

mock "tfplan/v2" {

}

And I did miss that difference indeed (sorry), but also now that I did read it, I still think we should approach it a bit differently, for the reasons I tried to explain above, which is that HCL (grammar) should be basically a superset of the individual HCL "flavours" in terms of structures and language constructs.

To put it another way - I would treat missing / as a bug in the existing HCL generic grammar. So we could add it there and then instead override it in Terraform grammar, which is a bit more restrictive?

That solves two problems:

1. You don't seem to intend to add any more keywords at this point - such as mock (correct me if I'm wrong), so you may just use the (fixed) generic HCL grammar to highlight these files.
2. The HCL grammar is now more correct and ready to highlight any other (non-Sentinel, non-Terraform) languages which may also use / in block labels.

HCL actually allows almost any Unicode characters for labels, but in crafting the grammar regexes we were optimising more for pragmatism than 100% correctness, hence we missed the forward slash! 😅
https://github.com/hashicorp/hcl/blob/main/hclsyntax/spec.md#identifiers

But to be fair, that's not the concern of the grammar file though. The editor should be deciding what grammar to use for which file. This filetypes parameter is just a hint to the Textmate editor (not even a vscode hint)

You are absolutely right that this isn't a concern for the grammars, but the reason I asked that question is because the grammar will be used in an IDE (VS Code or any other) which will be concerned about that and it relates to my earlier question about how specific you want your grammars to be and how you end up mapping it these to files.
If you are happy with just correcting the / detection, then you may just use the generic HCL grammar and also leverage the existing HCL extension as it will just match *.hcl - i.e. both of your naming conventions for sentinel.hcl and test cases *.hcl. If however you want to recognise particular block types (like mock, test, policy etc., like we do in the Terraform grammar, then I believe these should just be two different grammars, since the keywords will differ between test cases and the sentinel.hcl AFAICT?

[glennsarti]

Fair enough. I'll raise a new PR and allow / in block labels. Later I may resurrect this PR to only have the sentinel allowed block names

I believe these should just be two different grammars, since the keywords will differ between test cases and the sentinel.hcl AFAICT?

From a grammar perspective; test case files and config files are the same. Sentinel (the application) will just "ignore" the stuff that doesn't make sense. IMHO it's not the best, but it's what we have and may or may not change if/when we get to version 1.0.

Anyway, thanks for the background and I'll raise another PR soon.