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

remove pinning of versions #84

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
4 changes: 0 additions & 4 deletions .github/dependabot.yml
Original file line number Diff line number Diff line change
@@ -1,9 +1,5 @@
version: 2
updates:
- package-ecosystem: docker
directory: "/atlas/templates/docker"
schedule:
interval: daily
- package-ecosystem: "github-actions"
directory: "/"
schedule:
Expand Down
27 changes: 13 additions & 14 deletions atlas/templates/Makefile.common.gotmpl
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
PROJECT_ROOT ?= $(PWD)
BUILD_PATH ?= bin
DOCKERFILE_PATH ?= $(CURDIR)/docker
GO_IMAGE ?= golang:1.14-alpine
GO_IMAGE ?= golang:alpine
GO_RUNNER ?= $(DOCKER_RUNNER) $(GO_IMAGE)

# configuration for image names
Expand All @@ -28,7 +28,7 @@ GENERATOR ?= $(DOCKER_RUNNER) $(DOCKER_GENERATOR)
export GOFLAGS ?= -mod=vendor
GO_CACHE ?= -pkgdir $(BUILD_PATH)/go-cache
GO_BUILD_FLAGS ?= $(GO_CACHE) -i -v
GO_TEST_FLAGS ?= -v -cover
GO_TEST_FLAGS ?= -v -cover
GO_PACKAGES ?= $(shell go list ./... | grep -v vendor)


Expand Down Expand Up @@ -57,15 +57,15 @@ all-atlas: vendor-atlas protobuf-atlas docker-atlas

.PHONY fmt: fmt-atlas
fmt-atlas:
@$(GO_RUNNER) go fmt $(GO_PACKAGES)
$(GO_RUNNER) go fmt $(GO_PACKAGES)

.PHONY test: test-atlas
test-atlas: fmt-atlas
$(GO_RUNNER) go test $(GO_TEST_FLAGS) $(GO_PACKAGES)

docker-atlas:
@docker build -f $(SERVER_DOCKERFILE) -t $(SERVER_IMAGE):$(IMAGE_VERSION) .
@docker image prune -f --filter label=stage=server-intermediate
docker build --pull -f $(SERVER_DOCKERFILE) -t $(SERVER_IMAGE):$(IMAGE_VERSION) .
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

docker image prune -f --filter label=stage=server-intermediate

.docker-$(IMAGE_NAME)-$(IMAGE_VERSION):
$(MAKE) docker-atlas
Expand All @@ -78,7 +78,7 @@ push-atlas: docker
ifndef IMAGE_REGISTRY
@(echo "Please set IMAGE_REGISTRY variable in Makefile.vars to use push command"; exit 1)
else
@docker push $(SERVER_IMAGE):$(IMAGE_VERSION)
docker push $(SERVER_IMAGE):$(IMAGE_VERSION)
endif

.push-$(IMAGE_NAME)-$(IMAGE_VERSION):
Expand All @@ -90,33 +90,32 @@ push: .push-$(IMAGE_NAME)-$(IMAGE_VERSION)

.PHONY protobuf: protobuf-atlas
protobuf-atlas:
@$(GENERATOR) \
$(GENERATOR) \
$(PROTOBUF_ARGS) \
$(PROJECT_ROOT)/pkg/pb/service.proto

.PHONY vendor: vendor-atlas
vendor-atlas:
@go mod tidy
@go mod vendor
@go mod download
go mod tidy
go mod vendor

.PHONY clean: clean-atlas
clean-atlas:
@docker rmi -f $(shell docker images -q $(SERVER_IMAGE)) || true
docker rmi -f $(shell docker images -q $(SERVER_IMAGE)) || true
rm .push-* .docker-*

.PHONY migrate-up: migrate-up-atlas
migrate-up-atlas:
ifeq ($(WITH_DATABASE), true)
@$(DOCKER_RUNNER) --net="host" $(MIGRATETOOL_IMAGE) --verbose --path=$(MIGRATION_PATH_IN_CONTAINER)/ --database.dsn=$(DATABASE_URL) up
$(DOCKER_RUNNER) --net="host" $(MIGRATETOOL_IMAGE) --verbose --path=$(MIGRATION_PATH_IN_CONTAINER)/ --database.dsn=$(DATABASE_URL) up
else
@echo "Your application doesn't have database, migrations aren't supported"
endif

.PHONY migrate-down: migrate-down-atlas
migrate-down-atlas:
ifeq ($(WITH_DATABASE), true)
@$(DOCKER_RUNNER) --net="host" $(MIGRATETOOL_IMAGE) --verbose --path=$(MIGRATION_PATH_IN_CONTAINER)/ --database.dsn=$(DATABASE_URL) down
$(DOCKER_RUNNER) --net="host" $(MIGRATETOOL_IMAGE) --verbose --path=$(MIGRATION_PATH_IN_CONTAINER)/ --database.dsn=$(DATABASE_URL) down
else
@echo "Your application doesn't have database, migrations aren't supported"
endif
Expand Down Expand Up @@ -145,7 +144,7 @@ helm-archive:

.PHONY: helm-properties
helm-properties: helm/tpl.helm.properties
@sed 's/{CHART_FILE}/$(CHART_FILE)/g' helm/tpl.helm.properties > helm.properties
sed 's/{CHART_FILE}/$(CHART_FILE)/g' helm/tpl.helm.properties > helm.properties

.PHONY: push-chart
push-chart: helm-lint helm-archive helm-properties
Expand Down
2 changes: 1 addition & 1 deletion atlas/templates/docker/Dockerfile.gotmpl
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# build the server binary
FROM golang:1.15.7 AS builder
FROM golang:latest AS builder
Copy link
Contributor

@kd7lxl kd7lxl Mar 3, 2021

Choose a reason for hiding this comment

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

This doesn't lead to reproducible builds.

An alternative approach that would keep versions from going stale but also support reproducible builds would be to add a .github/dependabot.yml to the templates. Edit: implemented in #90

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should always be fixing bugs on the latest versions of dependencies. Dependabot PR would just fail/get ignored and continue the current process of creating apps on old versions of tools.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since go has been maintaining backwards compatibility for a decade, the only errors I expect to get here are with go test linting rules. Those find bad code so the risk here is quite low while the upsides are high.

Copy link
Contributor

Choose a reason for hiding this comment

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

the problem with removing the version is that it becomes ambiguous as to which version we are using. Tracing it back to a build would be a PITA. Perhaps we can write a build rule that would check what the latest release is and fail to build unless the user updates it. In any case, let's remove this change and deal with it in a separate PR.

LABEL stage=server-intermediate
WORKDIR /go/src/{{ .Root }}/{{ .Name }}

Expand Down