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 grpc-teleterm Make target and Dockerfile-teleterm #20032

Merged
merged 9 commits into from
Jan 26, 2023
Merged
29 changes: 0 additions & 29 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@ with_bpf := yes
BPF_TAG := bpf
BPF_MESSAGE := "with-BPF-support"
CLANG ?= $(shell which clang || which clang-10)
CLANG_FORMAT ?= $(shell which clang-format || which clang-format-10)
LLVM_STRIP ?= $(shell which llvm-strip || which llvm-strip-10)
KERNEL_ARCH := $(shell uname -m | sed 's/x86_64/x86/')
INCLUDES :=
Expand Down Expand Up @@ -225,8 +224,6 @@ export

TEST_LOG_DIR = ${abspath ./test-logs}

CLANG_FORMAT_STYLE = '{ColumnLimit: 100, IndentWidth: 4, Language: Proto}'

# Is this build targeting the same OS & architecture it is being compiled on, or
# will it require cross-compilation? We need to know this (especially for ARM) so we
# can set the cross-compiler path (and possibly feature flags) correctly.
Expand Down Expand Up @@ -1001,12 +998,6 @@ enter-root:
enter/centos7:
make -C build.assets enter/centos7

# Interactively enters a Docker container (which you can build and run Teleport Connect inside of).
# Similar to `enter`, but uses the teleterm container.
.PHONY:enter/teleterm
enter/teleterm:
make -C build.assets enter/teleterm


BUF := buf

Expand Down Expand Up @@ -1053,26 +1044,6 @@ grpc/host: protos/all
print/env:
env

# grpc-teleterm generates Go, TypeScript and JavaScript gRPC stubs from definitions for Teleport
# Terminal. This target runs in the buildbox-teleterm container.
#
# It exists as a separate target because on M1 MacBooks we must build grpc_node_plugin from source.
# That involves apt-get install of cmake & build-essential as well pulling hundreds of megabytes of
# git repos. It would significantly increase the time it takes to build buildbox for M1 users that
# don't need to generate Teleterm gRPC files.
# TODO(ravicious): incorporate grpc-teleterm into grpc once grpc-tools adds arm64 binary.
# https://github.com/grpc/grpc-node/issues/1405
.PHONY: grpc-teleterm
grpc-teleterm:
$(MAKE) -C build.assets grpc-teleterm

# grpc-teleterm/host generates GRPC stubs.
# Unlike grpc-teleterm, this target runs locally.
.PHONY: grpc-teleterm/host
grpc-teleterm/host: protos/all
$(BUF) generate --template=lib/prehog/buf-teleterm.gen.yaml lib/prehog/proto
$(BUF) generate --template=lib/teleterm/buf.gen.yaml lib/teleterm/api/proto

.PHONY: goinstall
goinstall:
go install $(BUILDFLAGS) \
Expand Down
64 changes: 57 additions & 7 deletions build.assets/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,11 @@
# teleport built on any newer Ubuntu version will not run on Centos 7 because
# of this.

ARG RUST_VERSION
Copy link
Member Author

Choose a reason for hiding this comment

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

RUST_VERSION was defined twice in Dockerfile.

# GRPC_NODE_PLUGIN_BINARY_TYPE has to be defined above the first FROM as it's
# used in another FROM instruction. See the comment for grpc_node_plugin image
# for more details.
# Valid values are "prebuilt" and "compiled".
ARG GRPC_NODE_PLUGIN_BINARY_TYPE

## LIBFIDO2 ###################################################################

Expand Down Expand Up @@ -68,6 +72,33 @@ RUN mkdir -p /opt && cd /opt && curl -L https://github.com/gravitational/libbpf/
make && \
make install

## GRPC_NODE_PLUGIN ###########################################################

# grpc_node_plugin is built only on arm64.
#
# To generate JS protofiles, we need the grpc-tools npm package. Unfortunately,
# it doesn't ship with a prebuilt grpc_node_plugin binary for Linux arm64. [1]
#
# We have to build it from source. Cloning all submodules takes a lot of time
# and bandwith so we build the binary in a separate image.
#
# [1] https://github.com/grpc/grpc-node/issues/1405#issuecomment-1282201879

FROM buildpack-deps:18.04 AS grpc_node_plugin
RUN apt-get update && \
apt-get install -y --no-install-recommends cmake
COPY teleterm_linux_arm64.toolchain.cmake ./linux_arm64.toolchain.cmake
RUN git clone --depth=1 --branch=grpc-tools@1.11.2 https://github.com/grpc/grpc-node.git && \
mv linux_arm64.toolchain.cmake grpc-node/packages/grpc-tools/. && \
cd grpc-node && \
git submodule update --init --recursive && \
cd packages/grpc-tools && \
cmake -DCMAKE_TOOLCHAIN_FILE=linux_arm64.toolchain.cmake . && \
cmake --build . --target clean && cmake --build . --target grpc_node_plugin -- -j 12 && \
mv grpc_node_plugin ../../../. && \
cd ../../.. && \
rm -rf grpc-node

## BUILDBOX ###################################################################

FROM ubuntu:18.04 AS buildbox
Expand Down Expand Up @@ -122,6 +153,8 @@ RUN apt-get -y update && \
python3-setuptools \
python3-wheel \
pkg-config \
# Used during tag builds to build the RPM package of Connect.
rpm \
softhsm2 \
sudo \
tree \
Expand Down Expand Up @@ -236,16 +269,16 @@ RUN make -C /opt/pam_teleport install

ENV SOFTHSM2_PATH "/usr/lib/softhsm/libsofthsm2.so"

# Install Nodejs
# Install Node.js
ARG NODE_VERSION
ENV NODE_URL="https://nodejs.org/dist/v${NODE_VERSION}/node-v${NODE_VERSION}-linux-${BUILDARCH}.tar.xz"
ENV NODE_PATH="/usr/local/lib/nodejs-linux"
ENV PATH="$PATH:${NODE_PATH}/bin"
RUN export NODE_ARCH=$(if [ "$BUILDARCH" = "amd64" ]; then echo "x64"; else echo "arm64"; fi) && \
export NODE_URL="https://nodejs.org/dist/v${NODE_VERSION}/node-v${NODE_VERSION}-linux-${NODE_ARCH}.tar.xz" && \
mkdir -p ${NODE_PATH} && \
curl -o /tmp/nodejs.tar.xz -L ${NODE_URL} && \
tar -xJf /tmp/nodejs.tar.xz -C /usr/local/lib/nodejs-linux --strip-components=1
export NODE_URL="https://nodejs.org/dist/v${NODE_VERSION}/node-v${NODE_VERSION}-linux-${NODE_ARCH}.tar.xz" && \
mkdir -p ${NODE_PATH} && \
curl -o /tmp/nodejs.tar.xz -L ${NODE_URL} && \
tar -xJf /tmp/nodejs.tar.xz -C /usr/local/lib/nodejs-linux --strip-components=1
RUN corepack enable yarn

# Install Rust
Expand Down Expand Up @@ -279,7 +312,7 @@ COPY --from=libbpf /usr/lib64/ /usr/lib64/
RUN cd /usr/local/lib && ldconfig

# Copy libfido2 libraries.
# Do this last to take better advantage of the multi-stage build.
# Do this near the end to take better advantage of the multi-stage build.
COPY --from=libfido2 /usr/local/include/ /usr/local/include/
COPY --from=libfido2 /usr/local/lib/pkgconfig/ /usr/local/lib/pkgconfig/
COPY --from=libfido2 \
Expand All @@ -294,5 +327,22 @@ RUN cd /usr/local/lib && \
ldconfig
COPY pkgconfig/buildbox/ /

# Install JS gRPC tools
RUN (npm install --global grpc_tools_node_protoc_ts@5.0.1)

FROM buildbox AS grpc_node_plugin_binary_prebuilt
ONBUILD RUN (npm install --global grpc-tools@1.11.2)

FROM buildbox AS grpc_node_plugin_binary_compiled
COPY --from=grpc_node_plugin grpc_node_plugin ./grpc_node_plugin
ONBUILD RUN npm install --global --ignore-scripts grpc-tools@1.11.2 && \
mv grpc_node_plugin $(npm root -g)/grpc-tools/bin/. && \
# grpc-tools needs protoc but we already install it a couple of steps above.
ln -s $(which protoc) $(npm root -g)/grpc-tools/bin/protoc

# A "conditional" FROM like this breaks up the number of steps in the progress bar of the given
# image, so let's use it at the end. This way the progress bar for buildbox stays mostly untouched.
FROM grpc_node_plugin_binary_${GRPC_NODE_PLUGIN_BINARY_TYPE} as buildbox
Copy link
Member Author

Choose a reason for hiding this comment

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

This conditional FROM is based on this SO answer: https://stackoverflow.com/a/60820156/742872


VOLUME ["/go/src/github.com/gravitational/teleport"]
EXPOSE 6600 2379 2380
54 changes: 0 additions & 54 deletions build.assets/Dockerfile-teleterm

This file was deleted.

40 changes: 5 additions & 35 deletions build.assets/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ buildbox:
--build-arg GOLANG_VERSION=$(GOLANG_VERSION) \
--build-arg RUST_VERSION=$(RUST_VERSION) \
--build-arg NODE_VERSION=$(NODE_VERSION) \
--build-arg GRPC_NODE_PLUGIN_BINARY_TYPE=$(GRPC_NODE_PLUGIN_BINARY_TYPE) \
--build-arg PROTOC_VER=$(PROTOC_VER) \
--build-arg GOGO_PROTO_TAG=$(GOGO_PROTO_TAG) \
--build-arg LIBBPF_VERSION=$(LIBBPF_VERSION) \
Expand Down Expand Up @@ -220,33 +221,17 @@ buildbox-arm-fips: buildbox-fips
--cache-from $(BUILDBOX_ARM_FIPS) \
--tag $(BUILDBOX_ARM_FIPS) -f Dockerfile-arm-fips .

#
# Builds a Docker buildbox which has tools needed to install grpc-tools npm package on arm64.
# See the `grpc-teleterm` target in the main Makefile for more details.
#
.PHONY:buildbox-teleterm
buildbox-teleterm: buildbox
@if [[ $${DRONE} == "true" ]] && ! docker inspect --type=image $(BUILDBOX_TELETERM) 2>&1 >/dev/null; then docker pull $(BUILDBOX_TELETERM) || true; fi;
docker build \
--build-arg BUILDBOX_VERSION=$(BUILDBOX_VERSION) \
--build-arg NODE_VERSION=$(NODE_VERSION) \
--build-arg BUILDARCH=$(RUNTIME_ARCH) \
--build-arg GRPC_NODE_PLUGIN_BINARY_TYPE=$(GRPC_NODE_PLUGIN_BINARY_TYPE) \
--cache-from $(BUILDBOX) \
--cache-from $(BUILDBOX_TELETERM) \
--tag $(BUILDBOX_TELETERM) -f Dockerfile-teleterm .

CONNECT_VERSION ?= $(VERSION)
ifeq ($(CONNECT_VERSION),)
CONNECT_VERSION := $(BUILDBOX_VERSION)-dev
endif

#
# Builds Teleport Connect inside the buildbox-teleterm container.
# Builds Teleport Connect inside the buildbox container.
#
.PHONY:teleterm
teleterm: buildbox-teleterm
docker run $(DOCKERFLAGS) $(NOROOT) $(BUILDBOX_TELETERM) \
teleterm: buildbox
docker run $(DOCKERFLAGS) $(NOROOT) $(BUILDBOX) \
bash -c "cd $(SRCDIR) && export CONNECT_TSH_BIN_PATH=\$$PWD/../teleport/build/tsh && yarn install --frozen-lockfile && yarn build-term && yarn package-term -c.extraMetadata.version=$(CONNECT_VERSION)"

# Builds webassets inside Docker.
Expand All @@ -259,16 +244,9 @@ ui: buildbox
.PHONY: grpc
grpc: buildbox
docker run \
$(DOCKERFLAGS) -e CLANG_FORMAT=/usr/bin/clang-format-10 -t $(BUILDBOX) \
$(DOCKERFLAGS) -t $(BUILDBOX) \
make -C /go/src/github.com/gravitational/teleport grpc/host

# grpc-teleterm generates GRPC stubs for Teleterm from inside buildbox-teleterm
.PHONY: grpc-teleterm
grpc-teleterm: buildbox-teleterm
docker run \
$(DOCKERFLAGS) -e CLANG_FORMAT=/usr/bin/clang-format-10 -t $(BUILDBOX_TELETERM) \
make -C /go/src/github.com/gravitational/teleport grpc-teleterm/host

# fix-imports runs GCI to sort and re-order Go imports in a deterministic way.
.PHONY: fix-imports
fix-imports: buildbox
Expand Down Expand Up @@ -380,14 +358,6 @@ enter/centos7: buildbox
docker run $(DOCKERFLAGS) -ti $(NOROOT) \
-e HOME=$(SRCDIR)/build.assets -w $(SRCDIR) $(BUILDBOX_CENTOS7) /bin/bash

#
# Starts shell inside the teleterm container
#
.PHONY:enter/teleterm
enter/teleterm: buildbox-teleterm
docker run $(DOCKERFLAGS) -ti $(NOROOT) \
-e HOME=$(SRCDIR)/build.assets -w $(SRCDIR) $(BUILDBOX_TELETERM) /bin/bash

#
# Create a Teleport package using the build container.
# Don't use this target directly; call named Makefile targets like release-amd64.
Expand Down
11 changes: 10 additions & 1 deletion build.assets/genproto.sh
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ main() {
# <teleport-root>/github.com/gravitational/teleport/..., so we copy them to
# the correct relative path.
trap 'rm -fr github.com' EXIT # don't leave github.com/ behind
rm -fr api/gen/proto gen/proto # cleanup gen/proto folders
# cleanup gen/proto folders
rm -fr api/gen/proto gen/proto lib/teleterm/api/protogen lib/prehog/gen lib/prehog/gen-js

# Generate Gogo protos.
buf generate --template=buf-gogo.gen.yaml api/proto
Expand All @@ -26,6 +27,14 @@ main() {
--path=proto/teleport/lib/multiplexer/
buf generate --template=lib/prehog/buf.gen.yaml lib/prehog/proto

# Generate lib/teleterm & JS protos.
# TODO(ravicious): Refactor generating JS protos to follow the approach from above, that is have a
# separate call to generate Go protos and another for JS protos instead of having
# teleterm-specific buf.gen.yaml files.
# https://github.com/gravitational/teleport/pull/19774#discussion_r1061524458
buf generate --template=lib/prehog/buf-teleterm.gen.yaml lib/prehog/proto
buf generate --template=lib/teleterm/buf.gen.yaml lib/teleterm/api/proto
Comment on lines +30 to +36
Copy link
Member Author

Choose a reason for hiding this comment

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

Originally I wanted to do this in this PR. But the list of changes is already big enough so I'll handle that in a separate PR (which I'm working on at the moment).


cp -r github.com/gravitational/teleport/* .
}

Expand Down
1 change: 0 additions & 1 deletion build.assets/images.mk
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,4 @@ BUILDBOX_CENTOS7=$(BUILDBOX_BASE_NAME)-centos7:$(BUILDBOX_VERSION)
BUILDBOX_CENTOS7_FIPS=$(BUILDBOX_BASE_NAME)-centos7-fips:$(BUILDBOX_VERSION)
BUILDBOX_ARM=$(BUILDBOX_BASE_NAME)-arm:$(BUILDBOX_VERSION)
BUILDBOX_ARM_FIPS=$(BUILDBOX_BASE_NAME)-arm-fips:$(BUILDBOX_VERSION)
BUILDBOX_TELETERM=$(BUILDBOX_BASE_NAME)-teleterm:$(BUILDBOX_VERSION)
BUILDBOX_UI=$(BUILDBOX_BASE_NAME)-ui:$(BUILDBOX_VERSION)
13 changes: 1 addition & 12 deletions lib/prehog/buf-teleterm.gen.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,6 @@ managed:
go_package_prefix:
default: github.com/gravitational/teleport/lib/prehog/gen
plugins:
- name: go
path: lib/prehog/bin/protoc-gen-go
out: lib/prehog/gen
opt:
- paths=source_relative
- name: connect-go
path: lib/prehog/bin/protoc-gen-connect-go
out: lib/prehog/gen
opt:
- paths=source_relative

- name: js
out: lib/prehog/gen-js
opt:
Expand All @@ -27,4 +16,4 @@ plugins:
path: grpc_tools_node_protoc_plugin
- name: ts
out: lib/prehog/gen-js
opt: "service=grpc-node"
opt: "service=grpc-node"
41 changes: 0 additions & 41 deletions lib/teleterm/api/protogen/js/v1/usage_events_grpc_pb.d.ts

This file was deleted.