Skip to content
This repository has been archived by the owner on May 25, 2023. It is now read-only.

Prevent native packager bin-compat issues #169

Merged
Show file tree
Hide file tree
Changes from 3 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
1 change: 1 addition & 0 deletions build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ lazy val root = (project in file("."))
IO.write(versionFile, versionSource)

Seq(versionFile)

}

addSbtPlugin("com.typesafe.sbt" % "sbt-native-packager" % Versions.nativePackager)
Expand Down
4 changes: 2 additions & 2 deletions src/main/scala/com/lightbend/rp/sbtreactiveapp/App.scala
Original file line number Diff line number Diff line change
Expand Up @@ -444,10 +444,10 @@ case object BasicApp extends DeployableApp {
val log = streams.value.log
val execCommand = dockerExecCommand.value

publishDocker(execCommand, alias.versioned, log)
publishDocker(execCommand, NativePackagerCompat.versioned(alias), log)

if (dockerUpdateLatest.value) {
publishDocker(execCommand, alias.latest, log)
publishDocker(execCommand, NativePackagerCompat.latest(alias), log)
}
}))

Expand Down
15 changes: 5 additions & 10 deletions src/main/scala/com/lightbend/rp/sbtreactiveapp/Deploy.scala
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,14 @@

package com.lightbend.rp.sbtreactiveapp

import com.lightbend.rp.sbtreactiveapp.BasicApp._
import com.lightbend.rp.sbtreactiveapp.SbtReactiveAppPlugin.localImport._
import com.lightbend.rp.sbtreactiveapp.SbtReactiveAppPlugin._
import com.typesafe.sbt.SbtNativePackager
import com.typesafe.sbt.packager.docker
import com.typesafe.sbt.packager.docker.DockerPlugin.publishLocalDocker
import com.lightbend.rp.sbtreactiveapp.SbtReactiveAppPlugin.localImport._
import com.typesafe.sbt.packager.Keys.stage
import com.typesafe.sbt.packager.docker.DockerPlugin.publishLocalDocker
import sbt.Keys._
import sbt._

import scala.collection.immutable.Seq
import com.typesafe.sbt.packager.docker.DockerSupport
import Keys._

trait DeployableApp extends App {
private val installReactiveSandbox = new java.util.concurrent.atomic.AtomicBoolean(false)
Expand All @@ -51,7 +47,6 @@ trait DeployableApp extends App {
deployMinikubePlayHttpSecretKeyValue := "dev-minikube",
deploy := {
import complete.DefaultParsers._
import scala.sys.process._

val args = spaceDelimited("<arg>").parsed
val isPlagom = Set("play", "lagom").contains(appType.value)
Expand Down Expand Up @@ -175,7 +170,7 @@ trait DeployableApp extends App {

val rpArgs =
Vector(
dockerAlias.value.versioned,
NativePackagerCompat.versioned(dockerAlias.value),
"--env",
s"JAVA_OPTS=${javaOpts.mkString(" ")}") ++
(if (bootstrapEnabled) Vector("--akka-cluster-skip-validation", "--pod-controller-replicas", deployMinikubeAkkaClusterBootstrapContactPoints.value.toString) else Vector.empty) ++
Expand All @@ -190,7 +185,7 @@ trait DeployableApp extends App {
minikubeExec.getAbsolutePath +: dockerBuildCommand.value,
log)

log.info(s"Built image ${dockerAlias.value.versioned}")
log.info(s"Built image ${NativePackagerCompat.versioned(dockerAlias.value)}")

if (reactiveSandbox) {
// FIXME: Make tiller & reactive-sandbox names configurable
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/*
* Copyright 2017 Lightbend, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.lightbend.rp.sbtreactiveapp

import com.typesafe.sbt.packager.docker.DockerAlias

/**
* `sbt-native-packager` is not SemVer compliant and classes like `DockerAlias` break binary compatibility
* in patch releases. This object provides makes all 1.3.x versions of `sbt-native-packager` compatible.
*/
object NativePackagerCompat {

// removed from `sbt-native-packager` (https://github.com/sbt/sbt-native-packager/pull/1138/files#diff-6aa67d5df515952c884df8bad5ff2ac4L12)
def untagged(dockerAlias: DockerAlias): String =
dockerAlias.registryHost.map(_ + "/").getOrElse("") +
dockerAlias.username.map(_ + "/").getOrElse("") + dockerAlias.name

// removed from `sbt-native-packager` (https://github.com/sbt/sbt-native-packager/pull/1138/files#diff-6aa67d5df515952c884df8bad5ff2ac4L12)
def versioned(dockerAlias: DockerAlias): String = {
untagged(dockerAlias) + dockerAlias.tag.map(":" + _).getOrElse("")
}

// removed from `sbt-native-packager` (https://github.com/sbt/sbt-native-packager/pull/1138/files#diff-6aa67d5df515952c884df8bad5ff2ac4L12)
def latest(dockerAlias: DockerAlias) = s"${untagged(dockerAlias)}:latest"
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the main change in the PR. There's no automated test for it, unfortunately. The issue manifests when running deploy minikube.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that the part that sbt-native-packager participate in deploy minikube is just image creation, can we reproduce this by calling Docker/stage from the scripted test?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually you probably need Docker/publish to exercise alias.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, good call. We can add that on a separate PR

Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package com.lightbend.rp.sbtreactiveapp
import com.typesafe.sbt.packager.Keys._
import com.typesafe.sbt.packager.archetypes.scripts.BashStartScriptPlugin
import com.typesafe.sbt.packager.docker
import com.typesafe.sbt.packager.docker.DockerKeys
import sbt.{ Def, _ }
import sbt.Keys._
import sbt.plugins.JvmPlugin
Expand Down Expand Up @@ -92,7 +93,7 @@ object SbtReactiveAppPlugin extends AutoPlugin {
}
}

object localImport extends docker.DockerKeys
val localImport: DockerKeys = docker.DockerPlugin.autoImport
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the original change. The breaking changes in docker.DockerKeys prevents user sbt's from running. This PR adds a scripted test to assert this fix kicks in.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍


override def requires = SbtReactiveAppPluginAll && docker.DockerPlugin && BashStartScriptPlugin && SbtReactiveAppPluginAll

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
name := "hello"
scalaVersion := "2.11.12"

lazy val root = (project in file("."))
.enablePlugins(SbtReactiveAppPlugin)
.settings(
packageName in Docker := "hello-play",
httpIngressPorts := Seq(9000),
httpIngressPaths := Seq("/")
)

Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
sys.props.get("plugin.version") match {
case Some(x) => addSbtPlugin("com.lightbend.rp" % "sbt-reactive-app" % x)
case _ => sys.error("""|The system property 'plugin.version' is not defined.
|Specify this property using the scriptedLaunchOpts -D.""".stripMargin)
}

addSbtPlugin("com.typesafe.sbt" % "sbt-native-packager" % "1.3.15")
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
object Main extends App {
println("hello")
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# This test only checks `sbt` starts successfully into a shell.
# Since Play 2.6.21 there's a dependency issue between Play's sbt-plugin and sbt-reactive-app where both bring in
# incompatible versions of `sbt-native-packager`. Users adding both `sbt-reactive-app` and a direct or transitive
# dependency to an incompatible version of `sbt-reactive-app` would be DoS-ing themselves.
dwijnand marked this conversation as resolved.
Show resolved Hide resolved
# Just starting `sbt` is enough to assert the fixes for compat work.

> about
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@ sys.props.get("plugin.version") match {
|Specify this property using the scriptedLaunchOpts -D.""".stripMargin)
}

addSbtPlugin("com.typesafe.play" % "sbt-plugin" % "2.6.7")
addSbtPlugin("com.typesafe.play" % "sbt-plugin" % "2.6.21")