Introduces a new (alternative) behavior for dockerfile with a Simple Text Matcher/Replacer

[dduportal]

This PR is an attempt (and a proposal) to fix #136 .

• It introduces a new alternative behavior for the dockerfile plugin referenced as "Simple Text":
  • A text matching "startwith" is done to find ALL candidates in the provided Dockerfile (might be multiple instructions)
  • The replacement ONLY changes the candidate lines: no reformatting and no removed commented lines
  • The new behavior has a replacement logic depending on the instruction kind (FROM, ARG, etc.).
  • Example with ARG:

## Matches any instruction starting with `ARG HELM_VERSION`:
# Matches ✔ ARG HELM_VERSION=3.0.0
# Matches ✔ ARG HELM_VERSION=
# Matches ✔ ARG HELM_VERSION
# Matches ✔ ARG HELM_VERSION=3.0.0
# NO match ✗ ENV HELM_VERSION
conditions:
  isARGTERRAFORMVERSIONSet:
    name: Is there any ARG instruction starting with "HELM_VERSION"
    kind: dockerfile
    spec:
      file: Dockerfile
      instruction:
        keyword: "ARG"
        matcher: "HELM_VERSION"

# If Source is "0.14.5"
targets:
  ## Replacement for ARG only happens on the value (not on the key). All instructions matching are replaced
  ## Default values are changed to non default
  #  ARG TERRAFORM_VERSION=0.13.2 → ARG TERRAFORM_VERSION=0.14.5
  #  ARG TERRAFORM_VERSION= → ARG TERRAFORM_VERSION=0.14.5
  #  ARG TERRAFORM_VERSION → ARG TERRAFORM_VERSION=0.14.5
  #  ENV TERRAFORM_VERSION ✗ (NO MATCH)
  updateARGTERRAFORMVERSION:
    name: Update all ARG instructions starting with TERRAFORM_VERSION
    kind: dockerfile
    spec:
      file: Dockerfile
      instruction:
        keyword: "ARG"
        matcher: "TERRAFORM_VERSION"

• Example with FROM:

## Matches any instruction starting with `FROM golang`:
# Matches ✔ FROM golang
# Matches ✔ FROM golang:1.15.0
# Matches ✔ FROM golang:latest AS builder
# NO match ✗ ENV golang
conditions:
  isFROMgolangSet:
    name: Is there any FROM instruction starting with "golang
    kind: dockerfile
    spec:
      file: Dockerfile
      instruction:
        keyword: "FROM"
        matcher: "golang"

# If Source is "1.16.0-alpine"
targets:
  ## Replacement for FROM only happens on the image name (not on the alias if provided).
  ## Default tag are changed to non default
  #  FROM golang:1.15.8 → FROM golang:1.16.0-alpine
  #  FROM golang:1.15.8 AS builder → FROM golang:1.16.0-alpine AS builder 
  #  FROM golang:latest → FROM golang:1.16.0-alpine
  #  FROM golang → FROM golang:1.16.0-alpine
  #  ENV golang ✗ (NO MATCH)
  updateFROMgolangVersion:
    name: Update all FROM instructions starting with golang
    kind: dockerfile
    spec:
      file: Dockerfile
      instruction:
        keyword: "FROM"
        matcher: "golang"

• The previous behavior is kept as it: the plugin dockerfile detects automatically if the configuration item instructions: receives a string (previous behavior referenced as "Moby Parser") or with a map (new behavior "Simple Text Parser").

• Implementation is now having an interface of a DockerfileParser, with 2 implementations: mobyparser (previous behavior) or simpletextparser (new behavior)

  • A refactoring had been made on the package dockerfile to handle the initialization of a parser based on the YAML config:
    • Extensive tests have be added to ensure that both behaviors are working as expected.

--- Initial message, for historical reason.

It's work in progress but I'm opening it to give you an idea and validate the concept.

The idea is to get rid of the fully fledged Dockerfile Moby's parser, and use textual replacement instead, to avoid loosing the Dockerfile's comment and structure (line returns, etc.).

• It implements support for FROM and ARG instructions as first step (ENV and LABEL are coming soon, the other will depend on your validation)
• There might be potential factorization and more test to be done
• Example had been updated, but I'll have to update doc as well
• The keyword matcher is currently implemented with a strings.Hasprefix . It could be changed into a "startsWith:" keyword. I did it quick and dirty but there is room for improvement here

WDYT @olblak ?

[olblak]

Thanks for the PR, while I haven't had the time to test it yet, I definitely understand that the first attempt to automate Dockerfile is not ideal and I really appreciate this different approach.
But I don't like to idea of just throwing away the first attempt, it's already used in production and it "works".
Instead, I would create the type dockerfile@v2, with a deprecation warning.
This is done here:

• condition
• target

[dduportal]

Hello @olblak 👋 As we discussed together, this PR is not aiming at removing the previous behavior. The code had been updated to manage automatically the kind of parsing to be done:

• There is now the concept of a DockerfileParser, with 2 implementations: mobyparser (previous behavior) or simpletextparser (new behavior)

  • mobyparser is to be used when only a single Dockerfile instruction is targeted. It is really precise but is loosing the Dockerfile comment.
  • simpletextparser is to be used when you need to change 1 or more instructions from a single source value, in a single batch change. But it has less instructions supported (yet?).

• A refactoring had been made on the package dockerfile which handles the initialization of a parser based on the YAML config:

  • If the value of instruction: is a string, then the mobyparser is used
  • If the value of instruction: is a map than can be casted to a map[string]string, then the new behavior is used.

• Extensives tests have be added to ensure that both behaviors are working as expected.

I still need to polish this PR (mainly around logrus messages), but 80% of the work has been done an you can start reviewing to have an idea.

[olblak]

While testing this PR, I realised how the examples are outdated and now in a broken state

[olblak]

I spotted a regression, in a condition or target, with the moby syntax, if we set the key "value" to something that doesn't exist it returns a stacktrace
The following example is a wrong config that lead to a stacktrace as MIRRORBITS_VERSION doesn't exist while MIRRORBIT_VERSION does

conditions:
  isENVSet:
    name: Is the 2nd ENV instruction having a "keyword" set to "MIRRORBIT_VERSION"
    kind: dockerfile
    spec:
      file: Dockerfile
      Instruction: "ENV[0][0]"
      Value: "MIRRORBITS_VERSION"
    scm:
      github:
        user: "olblak"
        email: "updatecli@olblak.com"
        owner: "olblak"
        repository: "mirrorbits"
        token: {{ requiredEnv "GITHUB_TOKEN" }}
        username: "olblak"
        branch: "master"

With version : "0.0.36", it returns

CONDITIONS:
===========


✔ Instruction 'ENV[0][0]' from Dockerfile 'Dockerfile', is correctly set to 'MIRRORBIT_VERSION'

with the latest it returns

✗ Instruction "ENV[0][0]" is incorrectly set to "MIRRORBIT_VERSION" instead of "MIRRORBITS_VERSION"
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x9a30c3]

goroutine 1 [running]:
github.com/olblak/updateCli/pkg/plugins/docker/dockerfile/mobyparser.MobyParser.FindInstruction(0xc00011ef30, 0x9, 0xc0004b6020, 0x12, 0xc0004b8000, 0x9a0, 0xe00, 0xc00003c540)
	/home/olblak/Project/Updatecli/updatecli/pkg/plugins/docker/dockerfile/mobyparser/main.go:54 +0x263
github.com/olblak/updateCli/pkg/plugins/docker/dockerfile.(*Dockerfile).Condition(0xc0001550e0, 0xc000232b80, 0x6, 0xc000626720, 0x2b, 0xc0005269e8)
	/home/olblak/Project/Updatecli/updatecli/pkg/plugins/docker/dockerfile/condition.go:26 +0x16c
github.com/olblak/updateCli/pkg/plugins/docker/dockerfile.(*Dockerfile).ConditionFromSCM(0xc0001550e0, 0xc000232b80, 0x6, 0x198a740, 0xc0002a00f0, 0x0, 0x0, 0xc000232b80)
	/home/olblak/Project/Updatecli/updatecli/pkg/plugins/docker/dockerfile/condition.go:35 +0xcb
github.com/olblak/updateCli/pkg/core/engine/condition.(*Condition).Run(0xc000526b58, 0xc000232b80, 0x6, 0xc000232b80, 0x6, 0x0)
	/home/olblak/Project/Updatecli/updatecli/pkg/core/engine/condition/main.go:81 +0x3e2
github.com/olblak/updateCli/pkg/core/engine.RunConditions(0xc000527908, 0x0, 0x0, 0xc00011ece0)
	/home/olblak/Project/Updatecli/updatecli/pkg/core/engine/main.go:326 +0x345
github.com/olblak/updateCli/pkg/core/engine.(*Engine).Run(0x22ce820, 0x0, 0x0)
	/home/olblak/Project/Updatecli/updatecli/pkg/core/engine/main.go:250 +0x1629
github.com/olblak/updateCli/cmd.run(0x178226b, 0x4, 0x0, 0x0)
	/home/olblak/Project/Updatecli/updatecli/cmd/root.go:99 +0x1c5
github.com/olblak/updateCli/cmd.glob..func2(0x22b9b00, 0xc0002c3180, 0x0, 0x5)
	/home/olblak/Project/Updatecli/updatecli/cmd/diff.go:29 +0x14d
github.com/spf13/cobra.(*Command).execute(0x22b9b00, 0xc0002c3130, 0x5, 0x5, 0x22b9b00, 0xc0002c3130)
	/home/olblak/go/pkg/mod/github.com/spf13/cobra@v1.0.0/command.go:846 +0x2c2
github.com/spf13/cobra.(*Command).ExecuteC(0x22b8de0, 0x1945580, 0xc0004889a0, 0xc000587f78)
	/home/olblak/go/pkg/mod/github.com/spf13/cobra@v1.0.0/command.go:950 +0x375
github.com/spf13/cobra.(*Command).Execute(...)
	/home/olblak/go/pkg/mod/github.com/spf13/cobra@v1.0.0/command.go:887
github.com/olblak/updateCli/cmd.Execute()
	/home/olblak/Project/Updatecli/updatecli/cmd/root.go:39 +0x7a
main.main()
	/home/olblak/Project/Updatecli/updatecli/main.go:8 +0x25
zsh: exit 2     ./bin/updateCli diff --config examples/updateCli.d/dockerfile.tpl --values 

[dduportal]

@olblak 2 commits added:

• Adding a unit test reproducing the issue with the panic: aa529aa
• Fix proposal in a2749d6

[jlevesy]

👀

Just dropping by, feel free to disregard :)