bake: fix output handling for push

[crazy-max]

fixes #2325

This is a regression from #2302 where override for --push was changed to *.output=type=registry but should be kept with *.push=true as there is a special handling to only set push=true if type=image or type=registry is already defined for a target. This PR puts back this behavior.

The --load case is also not properly handled in v0.13.0 as it doesn't append but overrides the outputs (was the same in v0.12). For backward compat, we can keep this behavior for this patch but in follow-up we should handle --load the same way as --push.

Here is the repro:

target "foo" {
  output = [ "type=local,dest=out" ]
}

target "bar" {
}

v0.12

$ docker buildx bake --push --print foo bar

Output

{
  "group": {
    "default": {
      "targets": [
        "foo",
        "bar"
      ]
    }
  },
  "target": {
    "bar": {
      "context": ".",
      "dockerfile": "Dockerfile",
      "output": [
        "type=image,push=true"
      ]
    },
    "foo": {
      "context": ".",
      "dockerfile": "Dockerfile",
      "output": [
        "type=local,dest=out"
      ]
    }
  }
}

$ docker buildx bake --load --print foo bar

Output

{
  "group": {
    "default": {
      "targets": [
        "foo",
        "bar"
      ]
    }
  },
  "target": {
    "bar": {
      "context": ".",
      "dockerfile": "Dockerfile",
      "output": [
        "type=docker"
      ]
    },
    "foo": {
      "context": ".",
      "dockerfile": "Dockerfile",
      "output": [
        "type=docker"
      ]
    }
  }
}

$ docker buildx bake --push --load --print foo bar
ERROR: push and load may not be set together at the moment

With this PR

$ docker buildx bake --push --print foo bar

Output

{
  "group": {
    "default": {
      "targets": [
        "foo",
        "bar"
      ]
    }
  },
  "target": {
    "bar": {
      "context": ".",
      "dockerfile": "Dockerfile",
      "output": [
        "type=image,push=true"
      ]
    },
    "foo": {
      "context": ".",
      "dockerfile": "Dockerfile",
      "output": [
        "type=local,dest=out"
      ]
    }
  }
}

$ docker buildx bake --load --print foo bar

Output

{
  "group": {
    "default": {
      "targets": [
        "foo",
        "bar"
      ]
    }
  },
  "target": {
    "bar": {
      "context": ".",
      "dockerfile": "Dockerfile",
      "output": [
        "type=docker"
      ]
    },
    "foo": {
      "context": ".",
      "dockerfile": "Dockerfile",
      "output": [
        "type=docker"
      ]
    }
  }
}

$ docker buildx bake --push --load --print foo bar

Output

{
  "group": {
    "default": {
      "targets": [
        "foo",
        "bar"
      ]
    }
  },
  "target": {
    "bar": {
      "context": ".",
      "dockerfile": "Dockerfile",
      "output": [
        "type=docker"
      ]
    },
    "foo": {
      "context": ".",
      "dockerfile": "Dockerfile",
      "output": [
        "type=docker"
      ]
    }
  }
}

[davhdavh]

Thanks for the quick fixing

[tonistiigi]

Do we have a similar test already for load ? Or was the behavior different as I can see it uses type=docker

[crazy-max]

We don't currently have a *.load override with bake so either v0.12, v0.13 overrides the outputs atm. See me note in PR description about --load case with bake. I can open a PR with this enhancement so we have a consistent behavior with build. This was my previous attempt, see 5af9738 but would not be backward compat unfortunately.