Empty dockerExposedPorts should be okay

[dave-handy]

Expected behaviour

sbt docker:publishLocal publishes local docker image without warnings

Actual behaviour

sbt docker:publishLocal prints warnings

Information

Using sbt-native-packager 1.4.1 sbt version 1.2.6 building on MacOS
Docker version 19.03.2, build 6a30dfc

This warning may make sense as a default, but there should be a separate settings key to turn off the warning such as dockerNoExposedPorts := true. Alternately a check can be added that checks if the settings have been explicitly overridden to Nil.

sbt-native-packager/src/main/scala/com/typesafe/sbt/packager/docker/DockerPlugin.scala

Lines 569 to 587 in 9047b33

	private[this] def validateExposedPorts(ports: Seq[Int], udpPorts: Seq[Int]): Validation.Validator = () => {
	if (ports.isEmpty && udpPorts.isEmpty) {
	List(
	ValidationWarning(
	description = "There are no exposed ports for your docker image",
	howToFix = """| Configure the `dockerExposedPorts` or `dockerExposedUdpPorts` setting. E.g.
	|
	| // standard tcp ports
	| dockerExposedPorts ++= Seq(9000, 9001)
	|
	| // for udp ports
	| dockerExposedUdpPorts += 4444
	""".stripMargin
	)
	)
	} else {
	List.empty
	}
	}

[muuki88]

Thanks for you detailed request @dave-handy 😄

Does this cause any issue for you or is it just annoying 😉 The reason I ask this is because introducing new settings is always a maintenance liability. If this is really causing issue, I would prefer to think about a generic way to disable single validations, e.g.

validatePackageValidators -= validateExposedPorts

[dave-handy]

Now that I understand there's no way to suppress it, it's just annoying. I had to do some hunting in the code to see find it so some other users might be looking for the same thing. It kind of makes me want to set a port just to get rid of the message, even though that seems like a bad idea ;)

Here are some other options I thought of:

• Set either or both existing settings to Seq(-1) and filter them out before passing to docker.
• Set either or both to some new constant value that can have a specific negative value i.e. val NoPorts = Seq(-234) and filter it out before passing to docker.
• Alter the warning message to acknowledge that not all docker images need to expose ports.

I have heard it's possible to check if a setting has been overridden in build.sbt even if it is the same as the default, that would really be the best way to go, but I have no idea how to do that.

[muuki88]

Now that I understand there's no way to suppress it, it's just annoying

That's at least something. You can deploy. But in anger 😝

It kind of makes me want to set a port just to get rid of the message, even though that seems like a bad idea

I know that feeling to well 😉

So if you feel lucky you can disable the validation completely for your sanity with

validatePackageValidators := List.empty

[dave-handy]

Yep, that would work! Sounds great

[dave-handy]

I wasn't aware that validatePackageValidators was already a setting. It looks like it's easy to set it to empty as you say but the validators are anonymous, private classes...

[muuki88]

but the validators are anonymous, private classes...

They are functions, yes. This makes it non-trivial to remove them from the list. Options to achieve this would be

• store each function as an instance. This allows removing by reference
• make the validatePackageValidators a Map[String, Validator], which would allow removal by key

[michalmela]

Isn't

validatePackageValidators := List.empty

kind of throwing the baby out with the bathwater? Really no plans to make this more easily configurable?

[muuki88]

Kind of. If you would like to open a pull request making this easier to configure I'm happy to help and review 😃
I have no priority for this as

1. this is only a warning. So if this warning bothers you, you can disable it
2. validations are an addition to the documentation for lazy devs like me that don't want to google the docs. So they don't prevent you from deploying something broken in production.
3. docker plugin is super complex already. Making it more complex should add some real value for a broad audience

[asnare]

I just stumbled across this while looking at something unrelated, and hadn't figured out how to suppress this warning. I understand the plugin is complicated already and adding support. I liked your original suggestion about how this could work:

[…] I would prefer to think about a generic way to disable single validations, e.g.

validatePackageValidators -= validateExposedPorts

I haven't done anything much with plugins before, but this seems to be a case of "simply" exposing the private validation rules so they can be removed. Would you still accept a PR for this?

Although "merely" annoying, this particular validation fires for every service I build with SBT: marking ports via EXPOSE in the Dockerfile has turned out to be a bit of an anti-pattern.

[muuki88]

Hi @asnare

PRs always welcome 🤩

[frosforever]

Bit of hack for those that want to remove the warning via filtering the validators by their results. Something like:

    Docker / validatePackage := {
      Validation.runAndThrow(
        (Docker / validatePackageValidators).value
          .map(v => () => v.apply().filterNot(_.description.contains("no exposed ports for your docker image"))),
        streams.value.log
      )
    }

Obviously brittle if the description message changes in the future.