Skip to content

Commit

Permalink
refactor: replace statik with embed (#7339)
Browse files Browse the repository at this point in the history
* refactor: replace statik with embed
  • Loading branch information
ericzzzzzzz committed Apr 27, 2022
1 parent cad65db commit 2fc43f6
Show file tree
Hide file tree
Showing 18 changed files with 72 additions and 97 deletions.
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,5 @@ docs/node_modules
docs/themes
docs/package-lock.json
pkg/skaffold/output/debug.test
cmd/skaffold/app/cmd/statik/statik.go
fs/assets/*_generated
secrets/keys.json
12 changes: 6 additions & 6 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,11 @@ GO_BUILD_TAGS_windows = release
GO_BUILD_TAGS_darwin = release

ifneq "$(strip $(LOCAL))" "true"
override STATIK_FILES = cmd/skaffold/app/cmd/statik/statik.go
override EMBEDDED_FILES_CHECK = fs/assets/check.txt
endif

# when build for local development (`LOCAL=true make install` can skip license check)
$(BUILD_DIR)/$(PROJECT): $(STATIK_FILES) $(GO_FILES) $(BUILD_DIR)
$(BUILD_DIR)/$(PROJECT): $(EMBEDDED_FILES_CHECK) $(GO_FILES) $(BUILD_DIR)
$(eval ldflags = $(GO_LDFLAGS) $(patsubst %,-extldflags \"%\",$(LDFLAGS_$(GOOS))))
$(eval tags = $(GO_BUILD_TAGS_$(GOOS)) $(GO_BUILD_TAGS_$(GOOS)_$(GOARCH)))
GOOS=$(GOOS) GOARCH=$(GOARCH) CGO_ENABLED=1 \
Expand All @@ -91,7 +91,7 @@ install: $(BUILD_DIR)/$(PROJECT)
.PHONY: cross
cross: $(foreach platform, $(SUPPORTED_PLATFORMS), $(BUILD_DIR)/$(PROJECT)-$(platform))

$(BUILD_DIR)/$(PROJECT)-%: $(STATIK_FILES) $(GO_FILES) $(BUILD_DIR)
$(BUILD_DIR)/$(PROJECT)-%: $(EMBEDDED_FILES_CHECK) $(GO_FILES) $(BUILD_DIR)
$(eval os = $(firstword $(subst -, ,$*)))
$(eval arch = $(lastword $(subst -, ,$(subst .exe,,$*))))
$(eval ldflags = $(GO_LDFLAGS) $(patsubst %,-extldflags \"%\",$(LDFLAGS_$(os))))
Expand Down Expand Up @@ -183,7 +183,7 @@ release-lts-build:

.PHONY: clean
clean:
rm -rf $(BUILD_DIR) hack/bin $(STATIK_FILES)
rm -rf $(BUILD_DIR) hack/bin $(EMBEDDED_FILES_CHECK) fs/assets/*_generated/

.PHONY: build_deps
build_deps:
Expand Down Expand Up @@ -313,8 +313,8 @@ flags-dashboard:

# static files

$(STATIK_FILES): go.mod docs/content/en/schemas/*
hack/generate-statik.sh
$(EMBEDDED_FILES_CHECK): go.mod docs/content/en/schemas/*
hack/generate-embedded-files.sh

# run comparisonstats - ex: make COMPARISONSTATS_ARGS='usr/local/bin/skaffold /usr/local/bin/skaffold helm-deployment main.go "//per-dev-iteration-comment"' comparisonstats
.PHONY: comparisonstats
Expand Down
18 changes: 6 additions & 12 deletions cmd/skaffold/app/cmd/credits/export.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,36 +20,30 @@ import (
"context"
"fmt"
"io"
iofs "io/fs"
"io/ioutil"
"log"
"os"
"path"
"path/filepath"
"strings"

"github.com/rakyll/statik/fs"

"github.com/GoogleContainerTools/skaffold/cmd/skaffold/app/cmd/statik"
"github.com/GoogleContainerTools/skaffold/fs"
)

var Path string

// Export writes all the licenses and credit files to the `Path` folder.
func Export(ctx context.Context, out io.Writer) error {
statikFS, err := statik.FS()
if err != nil {
return fmt.Errorf("opening embedded filesystem: %w", err)
}

if err := fs.Walk(statikFS, "/skaffold-credits", func(filePath string, fileInfo os.FileInfo, err error) error {
newPath := path.Join(Path, strings.Replace(filePath, "skaffold-credits", "", 1))
if fileInfo.IsDir() {
if err := iofs.WalkDir(fs.AssetsFS, "assets/credits_generated", func(filePath string, dirEntry iofs.DirEntry, err error) error {
newPath := path.Join(Path, strings.Replace(filePath, "assets/credits_generated", "", 1))
if dirEntry.IsDir() {
err := os.Mkdir(newPath, 0755)
if err != nil && !os.IsExist(err) {
return fmt.Errorf("creating directory %q: %w", newPath, err)
}
} else {
file, err := statikFS.Open(filePath)
file, err := fs.AssetsFS.Open(filePath)
if err != nil {
return fmt.Errorf("opening %q in embedded filesystem: %w", filePath, err)
}
Expand Down
13 changes: 0 additions & 13 deletions cmd/skaffold/app/cmd/credits/statik/statik.go

This file was deleted.

12 changes: 3 additions & 9 deletions cmd/skaffold/app/cmd/schema/print.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,20 +22,14 @@ import (
"path"
"strings"

"github.com/rakyll/statik/fs"

"github.com/GoogleContainerTools/skaffold/cmd/skaffold/app/cmd/statik"
"github.com/GoogleContainerTools/skaffold/fs"
)

// Print prints the json schema for a given version.
func Print(out io.Writer, version string) error {
statikFS, err := statik.FS()
if err != nil {
return err
}
filename := path.Join("assets/schemas_generated", strings.TrimPrefix(version, "skaffold/")+".json")

path := path.Join("/schemas", strings.TrimPrefix(version, "skaffold/")+".json")
content, err := fs.ReadFile(statikFS, path)
content, err := fs.AssetsFS.ReadFile(filename)
if err != nil {
return fmt.Errorf("schema %q not found: %w", version, err)
}
Expand Down
11 changes: 5 additions & 6 deletions cmd/skaffold/app/cmd/schema/print_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,22 +18,21 @@ package schema

import (
"bytes"
"net/http"
"testing"

"github.com/GoogleContainerTools/skaffold/cmd/skaffold/app/cmd/statik"
"github.com/GoogleContainerTools/skaffold/fs"
"github.com/GoogleContainerTools/skaffold/testutil"
)

func TestPrint(t *testing.T) {
fs := &testutil.FakeFileSystem{
fakeFS := &testutil.FakeFileSystem{
Files: map[string][]byte{
"/schemas/v1.json": []byte("{SCHEMA}"),
"assets/schemas_generated/v1.json": []byte("{SCHEMA}"),
},
}

testutil.Run(t, "found", func(t *testutil.T) {
t.Override(&statik.FS, func() (http.FileSystem, error) { return fs, nil })
fs.AssetsFS = fakeFS

var out bytes.Buffer
err := Print(&out, "skaffold/v1")
Expand All @@ -43,7 +42,7 @@ func TestPrint(t *testing.T) {
})

testutil.Run(t, "not found", func(t *testutil.T) {
t.Override(&statik.FS, func() (http.FileSystem, error) { return fs, nil })
fs.AssetsFS = fakeFS

var out bytes.Buffer
err := Print(&out, "skaffold/v0")
Expand Down
2 changes: 1 addition & 1 deletion deploy/skaffold/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ WORKDIR /skaffold
FROM build as builder
ARG VERSION
COPY . .
RUN make clean out/skaffold VERSION=$VERSION && mv out/skaffold /usr/bin/skaffold && rm -rf secrets $SECRET cmd/skaffold/app/cmd/statik/statik.go
RUN make clean out/skaffold VERSION=$VERSION && mv out/skaffold /usr/bin/skaffold && rm -rf secrets $SECRET

FROM build as release
COPY --from=builder /usr/bin/skaffold /usr/bin/skaffold
Expand Down
2 changes: 1 addition & 1 deletion deploy/skaffold/Dockerfile.lts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ WORKDIR /skaffold
FROM build as builder
ARG VERSION
COPY . .
RUN make clean out/skaffold VERSION=$VERSION && mv out/skaffold /usr/bin/skaffold && rm -rf secrets $SECRET cmd/skaffold/app/cmd/statik/statik.go
RUN make clean out/skaffold VERSION=$VERSION && mv out/skaffold /usr/bin/skaffold && rm -rf secrets $SECRET

FROM build as release
COPY --from=builder /usr/bin/skaffold /usr/bin/skaffold
Expand Down
1 change: 1 addition & 0 deletions fs/assets/help.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
This file is to ensure assets folder is not empty so that we can embed the folder.
17 changes: 13 additions & 4 deletions cmd/skaffold/app/cmd/statik/fs.go → fs/fs.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,20 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

package statik
package fs

import "github.com/rakyll/statik/fs"
import (
"embed"
"io/fs"
)

// For testing
var (
FS = fs.New

//go:embed assets/*
//Assets embedded file system
//nolint https://github.com/golang/lint/issues/503
Assets embed.FS

// AssetsFS for testing
AssetsFS fs.ReadFileFS = Assets
)
1 change: 0 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ require (
github.com/otiai10/copy v1.6.0
github.com/pkg/browser v0.0.0-20180916011732-0a3d74bf9ce4
github.com/pkg/errors v0.9.1
github.com/rakyll/statik v0.1.7
github.com/rjeczalik/notify v0.9.3-0.20201210012515-e2a77dcc14cf
github.com/russross/blackfriday/v2 v2.1.0
github.com/segmentio/textio v1.2.0
Expand Down
2 changes: 1 addition & 1 deletion hack/boilerplate/boilerplate.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@


SKIPPED_DIRS = ["Godeps", "third_party", ".git", "vendor", "examples", "testdata", "node_modules", "codelab"]
SKIPPED_FILES = ["install-golint.sh", "skaffold.pb.go", "skaffold.pb.gw.go", "skaffold_grpc.pb.go", "enums.pb.go", "build.sh", "statik.go", "gitutil.go"]
SKIPPED_FILES = ["install-golint.sh", "skaffold.pb.go", "skaffold.pb.gw.go", "skaffold_grpc.pb.go", "enums.pb.go", "build.sh", "gitutil.go"]

parser = argparse.ArgumentParser()
parser.add_argument("filenames", help="list of files to check, all files if unspecified", nargs='*')
Expand Down
31 changes: 11 additions & 20 deletions hack/generate-statik.sh → hack/generate-embedded-files.sh
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,8 @@ DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"

SECRET=${SECRET:-${DIR}/../secrets}
BIN=${DIR}/bin
STATIK=${BIN}/statik
LICENSES=${BIN}/go-licenses

TMP_DIR=$(mktemp -d ${TMPDIR:-/tmp}/generate-statik.XXXXXX)
trap "rm -rf $TMP_DIR" EXIT

if [ -x "$(command -v go-licenses)" ]; then
# use go-licenses binary if it's installed on user's path
Expand All @@ -33,31 +30,25 @@ elif ! [ -x "$(command -v ${LICENSES})" ]; then
# See https://github.com/golang/go/issues/30515
# Also can't be easily installed from a vendor folder because it relies on non-go files
# from a dependency.
echo "Installing go-licenses"
pushd $(mktemp -d ${TMPDIR:-/tmp}/generate-statik.XXXXXX)
#TODO(marlongamez): unpin this version once we're able to build using go 1.16.x
go mod init tmp; GOBIN=${BIN} go get github.com/google/go-licenses@9376cf9847a05cae04f4589fe4898b9bce37e684
popd
echo "Installing go-licenses"
pushd $(mktemp -d ${TMPDIR:-/tmp}/generate-embedded.XXXXXX)
#TODO(marlongamez): unpin this version once we're able to build using go 1.16.x
go mod init tmp; GOBIN=${BIN} go get github.com/google/go-licenses@9376cf9847a05cae04f4589fe4898b9bce37e684
popd
fi

echo "Collecting licenses"
cd ${DIR}/..
${LICENSES} save github.com/GoogleContainerTools/skaffold/cmd/skaffold --save_path="${TMP_DIR}/skaffold-credits"
chmod -R u+w "${TMP_DIR}/skaffold-credits"
${LICENSES} save github.com/GoogleContainerTools/skaffold/cmd/skaffold --save_path="fs/assets/credits_generated" --force
chmod -R u+w "fs/assets/credits_generated"

echo "Collecting schemas"
cp -R docs/content/en/schemas "${TMP_DIR}/schemas"
cp -R docs/content/en/schemas "fs/assets/schemas_generated"

if ! [[ -f ${STATIK} ]]; then
echo 'Installing statik tool'
pushd ${DIR}/tools
GOBIN=${BIN} GO111MODULE=on go install -tags tools github.com/rakyll/statik
popd
fi

if [[ -d ${SECRET} ]]; then
echo "generating statik for secret"
cp -R ${SECRET} "${TMP_DIR}/secret"
echo "generating embedded files for secret"
cp -R ${SECRET} "fs/assets/secrets_generated"
fi

${STATIK} -f -src=${TMP_DIR} -m -dest cmd/skaffold/app/cmd
echo "Used for marking generating embedded files task complete, don't modify this." > fs/assets/check.txt
1 change: 0 additions & 1 deletion hack/tools/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,5 @@ go 1.14

require (
github.com/corneliusweig/release-notes v0.0.0-20191014214505-0be5c7c66752
github.com/rakyll/statik v0.1.6
golang.org/x/tools v0.1.5
)
1 change: 0 additions & 1 deletion hack/tools/tools.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,4 @@ package main

import (
_ "github.com/corneliusweig/release-notes"
_ "github.com/rakyll/statik"
)
9 changes: 2 additions & 7 deletions pkg/skaffold/instrumentation/export.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ import (
mexporter "github.com/GoogleCloudPlatform/opentelemetry-operations-go/exporter/metric"
texporter "github.com/GoogleCloudPlatform/opentelemetry-operations-go/exporter/trace"
"github.com/mitchellh/go-homedir"
"github.com/rakyll/statik/fs"
"go.opentelemetry.io/otel"
"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/exporters/stdout"
Expand All @@ -45,7 +44,7 @@ import (
"go.opentelemetry.io/otel/trace"
"google.golang.org/api/option"

"github.com/GoogleContainerTools/skaffold/cmd/skaffold/app/cmd/statik"
fs "github.com/GoogleContainerTools/skaffold/fs"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/output/log"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/user"
Expand Down Expand Up @@ -110,11 +109,7 @@ func exportMetrics(ctx context.Context, filename string, meter skaffoldMeter) er
}

func initCloudMonitoringExporterMetrics() (*basic.Controller, error) {
statikFS, err := statik.FS()
if err != nil {
return nil, err
}
b, err := fs.ReadFile(statikFS, "/secret/keys.json")
b, err := fs.AssetsFS.ReadFile("assets/secrets_generated/keys.json")
if err != nil {
// No keys have been set in this version so do not attempt to write metrics
if os.IsNotExist(err) {
Expand Down
19 changes: 9 additions & 10 deletions pkg/skaffold/instrumentation/export_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"encoding/json"
"fmt"
"io/ioutil"
"net/http"
"os"
"strings"
"testing"
Expand All @@ -30,7 +29,7 @@ import (
"go.opentelemetry.io/otel/exporters/stdout"
"go.opentelemetry.io/otel/sdk/metric/controller/basic"

"github.com/GoogleContainerTools/skaffold/cmd/skaffold/app/cmd/statik"
"github.com/GoogleContainerTools/skaffold/fs"
"github.com/GoogleContainerTools/skaffold/proto/v1"
"github.com/GoogleContainerTools/skaffold/testutil"
)
Expand Down Expand Up @@ -106,9 +105,9 @@ func TestExportMetrics(t *testing.T) {
Duration: time.Minute * 4,
}
metersBytes, _ := json.Marshal([]skaffoldMeter{buildMeter, devMeter, debugMeter})
fs := &testutil.FakeFileSystem{
fakeFS := testutil.FakeFileSystem{
Files: map[string][]byte{
"/secret/keys.json": []byte(testKey),
"assets/secrets_generated/keys.json": []byte(testKey),
},
}

Expand Down Expand Up @@ -166,7 +165,7 @@ func TestExportMetrics(t *testing.T) {
filename := "metrics"
openTelFilename := "otel_metrics"

t.Override(&statik.FS, func() (http.FileSystem, error) { return fs, nil })
fs.AssetsFS = fakeFS
t.Override(&isOnline, test.isOnline)

if test.isOnline {
Expand Down Expand Up @@ -217,7 +216,7 @@ func TestInitCloudMonitoring(t *testing.T) {
{
name: "if key present pusher is not nil",
fileSystem: &testutil.FakeFileSystem{
Files: map[string][]byte{"/secret/keys.json": []byte(testKey)},
Files: map[string][]byte{"assets/secrets_generated/keys.json": []byte(testKey)},
},
},
{
Expand All @@ -231,7 +230,7 @@ func TestInitCloudMonitoring(t *testing.T) {
name: "credentials without project_id returns an error",
fileSystem: &testutil.FakeFileSystem{
Files: map[string][]byte{
"/secret/keys.json": []byte(`{
"assets/secrets_generated/keys.json": []byte(`{
"client_id": "test_id",
"client_secret": "test_secret",
"refresh_token": "test_token",
Expand All @@ -245,7 +244,7 @@ func TestInitCloudMonitoring(t *testing.T) {
}
for _, test := range tests {
testutil.Run(t, test.name, func(t *testutil.T) {
t.Override(&statik.FS, func() (http.FileSystem, error) { return test.fileSystem, nil })
fs.AssetsFS = test.fileSystem

p, err := initCloudMonitoringExporterMetrics()

Expand All @@ -255,7 +254,7 @@ func TestInitCloudMonitoring(t *testing.T) {
}

func TestUserMetricReported(t *testing.T) {
fs := &testutil.FakeFileSystem{
fakeFS := &testutil.FakeFileSystem{
Files: map[string][]byte{
"/secret/keys.json": []byte(testKey),
},
Expand Down Expand Up @@ -370,7 +369,7 @@ func TestUserMetricReported(t *testing.T) {
filename := "metrics"
openTelFilename := "otel_metrics"

t.Override(&statik.FS, func() (http.FileSystem, error) { return fs, nil })
fs.AssetsFS = fakeFS
t.Override(&isOnline, true)
tmpFile, err := os.OpenFile(tmp.Path(openTelFilename), os.O_RDWR|os.O_CREATE, os.ModePerm)
if err != nil {
Expand Down

0 comments on commit 2fc43f6

Please sign in to comment.