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

strict macro linting #336

Merged
merged 2 commits into from
Jul 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion examples/linter/custom_linter.vcl
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ backend F_httpbin_org {
}

sub vcl_recv {
#Fastly recv
#FASTLY recv
set req.backend = F_httpbin_org;
return (lookup);
}
Expand Down
6 changes: 3 additions & 3 deletions examples/linter/default01.vcl
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ sub custom_logger {

sub vcl_recv {

#Fastly recv
#FASTLY recv
set req.backend = httpbin_org;
set req.http.Foo = {" foo bar baz "};
call custom_logger;
Expand All @@ -42,15 +42,15 @@ sub vcl_recv {

sub vcl_deliver {

#Fastly deliver
#FASTLY deliver
set resp.http.X-Custom-Header = "Custom Header";
call custom_logger;
return (deliver);
}

sub vcl_fetch {

#Fastly fetch
#FASTLY fetch
call custom_logger;
return(deliver);
}
6 changes: 3 additions & 3 deletions examples/linter/default02.vcl
Original file line number Diff line number Diff line change
Expand Up @@ -27,21 +27,21 @@ backend httpbin_org {

sub vcl_recv {

#Fastly recv
#FASTLY recv
set req.backend = httpbin_org;

return (pass);
}

sub vcl_deliver {

#Fastly deliver
#FASTLY deliver
set resp.http.X-Custom-Header = "Custom Header";
return (deliver);
}

sub vcl_fetch {

#Fastly fetch
#FASTLY fetch
return(deliver);
}
6 changes: 3 additions & 3 deletions examples/linter/default03.vcl
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ backend httpbin_org {

sub vcl_recv {

#Fastly recv
#FASTLY recv
set req.backend = httpbin_org;

if (req.http.Host !~ "(foo)\.example\.com") {
Expand All @@ -43,13 +43,13 @@ sub vcl_recv {

sub vcl_deliver {

#Fastly deliver
#FASTLY deliver
set resp.http.X-Custom-Header = "Custom Header";
return (deliver);
}

sub vcl_fetch {

#Fastly fetch
#FASTLY fetch
return(deliver);
}
8 changes: 4 additions & 4 deletions examples/linter/default04.vcl
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,15 @@ backend httpbin_org {

sub vcl_recv {

#Fastly recv
#FASTLY recv
set req.backend = httpbin_org;

return (pass);
}

sub vcl_deliver {

#Fastly deliver
#FASTLY deliver
set resp.http.X-Custom-Header = "Custom Header";
return (deliver);
}
Expand All @@ -44,13 +44,13 @@ sub vcl_fetch {

error 755 "/login?s=error";

#Fastly fetch
#FASTLY fetch
return(deliver);
}

sub vcl_error {

#Fastly error
#FASTLY error
if (obj.status == 755) {
set obj.http.Location = regsub(obj.response, "^([^;]*)(;.*)?$", "\1");
set obj.response = "Found";
Expand Down
2 changes: 1 addition & 1 deletion examples/linter/default05.vcl
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ include "default05_include01";

sub vcl_recv {

#Fastly recv
#FASTLY recv
set req.backend = httpbin_org;

if (req.http.Some-Truthy-Header) {
Expand Down
66 changes: 61 additions & 5 deletions linter/declaration_linter_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package linter

import "testing"
import (
"fmt"
"testing"
)

func TestLintAclDeclaration(t *testing.T) {
t.Run("pass", func(t *testing.T) {
Expand Down Expand Up @@ -302,7 +305,7 @@ sub example {
t.Run("pass with Fastly reserved subroutine boilerplate comment", func(t *testing.T) {
input := `
sub vcl_recv {
# FASTLY recv
#FASTLY recv
set req.http.Host = "example.com";
}`
assertNoError(t, input)
Expand All @@ -311,7 +314,7 @@ sub vcl_recv {
sub vcl_log {
# FASTLY log
}`
assertNoError(t, input)
assertError(t, input)
})

t.Run("invalid subroutine name", func(t *testing.T) {
Expand Down Expand Up @@ -359,12 +362,12 @@ sub example {
}

sub vcl_log {
# FASTLY log
#FASTLY log
call example;
}

sub vcl_recv {
# FASTLY recv
#FASTLY recv
call example;
}
`
Expand Down Expand Up @@ -566,3 +569,56 @@ sub test_sub{
assertError(t, input)
})
}

func TestFastlyBoilerPlateMacro(t *testing.T) {
tests := []struct {
name string
macro string
isError bool
}{
{
name: "Disallow slash comment sign",
macro: "//FASTLY RECV",
isError: true,
},
{
name: "Disallow double or more comment sign",
macro: "###FASTLY RECV",
isError: true,
},
{
name: "Disallow lowercase fastly string",
macro: "#fastly RECV",
isError: true,
},
{
name: "Allow uppercase scope",
macro: "#FASTLY RECV",
},
{
name: "Allow lowercase scope",
macro: "#FASTLY recv",
},
{
name: "Allow extra comments",
macro: "#FASTLY RECV foo bar baz",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
input := fmt.Sprintf(`
sub vcl_recv {
%s
set req.http.Foo = "bar";
}`,
tt.macro,
)
if tt.isError {
assertError(t, input)
} else {
assertNoError(t, input)
}
})
}
}
14 changes: 7 additions & 7 deletions linter/expression_linter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ sub foo {
t.Run("pass: with argument", func(t *testing.T) {
input := `
sub vcl_recv {
#Fastly recv
#FASTLY recv
return (pass);
}`
assertNoError(t, input)
Expand All @@ -211,7 +211,7 @@ sub vcl_recv {
t.Run("pass: with reserved word", func(t *testing.T) {
input := `
sub vcl_recv {
#Fastly recv
#FASTLY recv
return (restart);
}`
assertNoError(t, input)
Expand All @@ -220,7 +220,7 @@ sub vcl_recv {
t.Run("sub: return correct type", func(t *testing.T) {
input := `
sub custom_sub INTEGER {
#Fastly recv
#FASTLY recv
return 1;
}`
assertNoError(t, input)
Expand Down Expand Up @@ -303,7 +303,7 @@ sub get_bool BOOL {
func TestBlockSyntaxInsideBlockStatement(t *testing.T) {
input := `
sub vcl_recv {
#Fastly recv
#FASTLY recv
{
log "vcl_recv";
}
Expand All @@ -314,7 +314,7 @@ sub vcl_recv {
func TestBlockSyntaxInsideBlockStatementmultiply(t *testing.T) {
input := `
sub vcl_recv {
#Fastly recv
#FASTLY recv
{
{
log "vcl_recv";
Expand All @@ -328,7 +328,7 @@ func TestRegexExpressionIsInvalid(t *testing.T) {
t.Run("pass", func(t *testing.T) {
input := `
sub vcl_recv {
#Fastly recv
#FASTLY recv
if (req.url ~ "^/([^\?]*)?(\?.*)?$") {
restart;
}
Expand All @@ -339,7 +339,7 @@ sub vcl_recv {
t.Run("error: invalid regex", func(t *testing.T) {
input := `
sub vcl_recv {
#Fastly recv
#FASTLY recv
if (req.url ~ "^/([^\?]*)?(\?.*?$") {
restart;
}
Expand Down
15 changes: 12 additions & 3 deletions linter/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -438,10 +438,19 @@ func getFastlySubroutineScope(name string) string {
return ""
}

func hasFastlyBoilerPlateMacro(cs ast.Comments, phrase string) bool {
// Fastly macro format Must be "#FASTLY [scope]"
// - Comment sign must starts with single "#". "/" sign is not accepted
// - Fixed "FASTLY" string must exactly present without whitespace after comment sign
// - [scope] string is case-insensitive (recv/RECV can be accepcted, typically uppercase)
// - Additional comment is also accepted like "#FASTLY RECV some extra comment"
func hasFastlyBoilerPlateMacro(cs ast.Comments, scope string) bool {
for _, c := range cs {
line := strings.TrimLeft(c.String(), " */#")
if strings.HasPrefix(strings.ToUpper(line), phrase) {
// Uppercase scope
if strings.HasPrefix(c.String(), "#FASTLY "+strings.ToUpper(scope)) {
return true
}
// lowercase scope
if strings.HasPrefix(c.String(), "#FASTLY "+strings.ToLower(scope)) {
return true
}
}
Expand Down
8 changes: 3 additions & 5 deletions linter/linter.go
Original file line number Diff line number Diff line change
Expand Up @@ -549,8 +549,6 @@ func (l *Linter) factoryRootDeclarations(statements []ast.Statement, ctx *contex
}

func (l *Linter) lintFastlyBoilerPlateMacro(sub *ast.SubroutineDeclaration, ctx *context.Context, scope string) {
phrase := strings.ToUpper("FASTLY " + scope)

// prepare scoped snippets
scopedSnippets, ok := ctx.Snippets().ScopedSnippets[scope]
if !ok {
Expand All @@ -559,7 +557,7 @@ func (l *Linter) lintFastlyBoilerPlateMacro(sub *ast.SubroutineDeclaration, ctx

var resolved []ast.Statement
// visit all statement comments and find "FASTLY [phase]" comment
if hasFastlyBoilerPlateMacro(sub.Block.Infix, phrase) {
if hasFastlyBoilerPlateMacro(sub.Block.Infix, scope) {
for _, s := range scopedSnippets {
resolved = append(resolved, l.loadSnippetVCL("snippet::"+s.Name, s.Data)...)
}
Expand All @@ -569,7 +567,7 @@ func (l *Linter) lintFastlyBoilerPlateMacro(sub *ast.SubroutineDeclaration, ctx

var found bool
for _, stmt := range sub.Block.Statements {
if hasFastlyBoilerPlateMacro(stmt.GetMeta().Leading, phrase) && !found {
if hasFastlyBoilerPlateMacro(stmt.GetMeta().Leading, scope) && !found {
// Macro found but embedding snippets should do only once
for _, s := range scopedSnippets {
resolved = append(resolved, l.loadSnippetVCL("snippet::"+s.Name, s.Data)...)
Expand All @@ -590,7 +588,7 @@ func (l *Linter) lintFastlyBoilerPlateMacro(sub *ast.SubroutineDeclaration, ctx
Severity: WARNING,
Token: sub.GetMeta().Token,
Message: fmt.Sprintf(
`Subroutine "%s" is missing Fastly boilerplate comment "%s" inside definition`, sub.Name.Value, phrase,
`Subroutine "%s" is missing Fastly boilerplate comment "#FASTLY %s" inside definition`, sub.Name.Value, strings.ToUpper(scope),
),
}
l.Error(err.Match(SUBROUTINE_BOILERPLATE_MACRO))
Expand Down
8 changes: 4 additions & 4 deletions linter/linter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,14 +128,14 @@ func TestLintStuff(t *testing.T) {
}

sub vcl_log {
# FASTLY log
#FASTLY log
if (example()) {
log "foo";
}
}

sub vcl_deliver {
# FASTLY deliver
#FASTLY deliver
if (example()) {
log "foo";
}
Expand Down Expand Up @@ -398,7 +398,7 @@ func TestPassIssue39(t *testing.T) {
t.Run("pass", func(t *testing.T) {
input := `
sub vcl_fetch {
### FASTLY fetch
#FASTLY fetch
if (parse_time_delta(beresp.http.Edge-Control:cache-maxage) >= 0) {
set beresp.ttl = parse_time_delta(beresp.http.Edge-Control:cache-maxage);
}
Expand All @@ -413,7 +413,7 @@ func TestSubroutineHoisting(t *testing.T) {
t.Run("pass", func(t *testing.T) {
input := `
sub vcl_recv {
### FASTLY recv
#FASTLY recv
call hoisted_subroutine;
return(lookup);
}
Expand Down