From e509114d33504c217a62ecc62e8e09370b51c1b0 Mon Sep 17 00:00:00 2001 From: Jeff Widman Date: Fri, 12 Aug 2022 02:07:00 -0600 Subject: [PATCH] Remove the native `go` helper There are two functions that do the same thing, one in `ruby` and one in `go`. TODO explain why we had them both and why safe to remove the `go` one once I finish researching it. So I removed the one written in `go`. It turns out this function was the only reason we had native `go` helpers, so I was able to completely remove the helpers infra altogether. Fix https://github.com/dependabot/dependabot-core/issues/4448 --- .github/dependabot.yml | 4 -- Dockerfile | 3 - bin/docker-dev-shell | 1 - go_modules/helpers/Makefile | 9 --- go_modules/helpers/build | 28 -------- go_modules/helpers/go.mod | 5 -- go_modules/helpers/go.sum | 2 - go_modules/helpers/importresolver/main.go | 34 ---------- go_modules/helpers/main.go | 67 ------------------ .../dependabot/go_modules/metadata_finder.rb | 3 +- .../dependabot/go_modules/path_converter.rb | 20 +----- .../go_modules/path_converter_spec.rb | 68 +++++++------------ 12 files changed, 29 insertions(+), 215 deletions(-) delete mode 100644 go_modules/helpers/Makefile delete mode 100755 go_modules/helpers/build delete mode 100644 go_modules/helpers/go.mod delete mode 100644 go_modules/helpers/go.sum delete mode 100644 go_modules/helpers/importresolver/main.go delete mode 100644 go_modules/helpers/main.go diff --git a/.github/dependabot.yml b/.github/dependabot.yml index c9b5c287b35..88dc571b2a6 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -28,10 +28,6 @@ updates: directory: "/" schedule: interval: "weekly" - - package-ecosystem: "gomod" - directory: "/go_modules/helpers" - schedule: - interval: "weekly" - package-ecosystem: "mix" directory: "/hex/helpers" schedule: diff --git a/Dockerfile b/Dockerfile index 28eaf74cb29..9f1bf4b1913 100644 --- a/Dockerfile +++ b/Dockerfile @@ -303,9 +303,6 @@ COPY --chown=dependabot:dependabot bundler/helpers /opt/bundler/helpers RUN bash /opt/bundler/helpers/v1/build \ && bash /opt/bundler/helpers/v2/build -COPY --chown=dependabot:dependabot go_modules/helpers /opt/go_modules/helpers -RUN bash /opt/go_modules/helpers/build - COPY --chown=dependabot:dependabot hex/helpers /opt/hex/helpers ENV MIX_HOME="/opt/hex/mix" # https://github.com/hexpm/hex/releases diff --git a/bin/docker-dev-shell b/bin/docker-dev-shell index d5b5e391f79..cfac4dd4616 100755 --- a/bin/docker-dev-shell +++ b/bin/docker-dev-shell @@ -191,7 +191,6 @@ docker run --rm -ti \ -v "$(pwd)/go_modules/.rubocop.yml:$CODE_DIR/go_modules/.rubocop.yml" \ -v "$(pwd)/go_modules/Gemfile:$CODE_DIR/go_modules/Gemfile" \ -v "$(pwd)/go_modules/dependabot-go_modules.gemspec:$CODE_DIR/go_modules/dependabot-go_modules.gemspec" \ - -v "$(pwd)/go_modules/helpers:$CODE_DIR/go_modules/helpers" \ -v "$(pwd)/go_modules/lib:$CODE_DIR/go_modules/lib" \ -v "$(pwd)/go_modules/spec:$CODE_DIR/go_modules/spec" \ -v "$(pwd)/go_modules/script:$CODE_DIR/go_modules/script" \ diff --git a/go_modules/helpers/Makefile b/go_modules/helpers/Makefile deleted file mode 100644 index 1d5994f7005..00000000000 --- a/go_modules/helpers/Makefile +++ /dev/null @@ -1,9 +0,0 @@ -.PHONY = all - -all: darwin linux - -darwin: - GOOS=darwin GOARCH=amd64 go build -o go-helpers.darwin64 . - -linux: - GOOS=linux GOARCH=amd64 go build -o go-helpers.linux64 . diff --git a/go_modules/helpers/build b/go_modules/helpers/build deleted file mode 100755 index 0833055ed06..00000000000 --- a/go_modules/helpers/build +++ /dev/null @@ -1,28 +0,0 @@ -#!/bin/bash - -set -e - -if [ -z "$DEPENDABOT_NATIVE_HELPERS_PATH" ]; then - echo "Unable to build, DEPENDABOT_NATIVE_HELPERS_PATH is not set" - exit 1 -fi - -install_dir="$DEPENDABOT_NATIVE_HELPERS_PATH/go_modules" - -if ! [[ "$install_dir" =~ ^/ ]]; then - echo "$install_dir must be an absolute path" - exit 1 -fi - -if [ ! -d "$install_dir/bin" ]; then - mkdir -p "$install_dir/bin" -fi - -helpers_dir="$(dirname "${BASH_SOURCE[0]}")" -cd "$helpers_dir" - -os="$(uname -s | tr '[:upper:]' '[:lower:]')" -echo "building $install_dir/bin/helper" - -GOOS="$os" GOARCH=amd64 go build -o "$install_dir/bin/helper" . -go clean -cache -modcache diff --git a/go_modules/helpers/go.mod b/go_modules/helpers/go.mod deleted file mode 100644 index 5ac2fa0a94e..00000000000 --- a/go_modules/helpers/go.mod +++ /dev/null @@ -1,5 +0,0 @@ -module github.com/dependabot/dependabot-core/go_modules/helpers - -go 1.19 - -require github.com/Masterminds/vcs v1.13.3 diff --git a/go_modules/helpers/go.sum b/go_modules/helpers/go.sum deleted file mode 100644 index 89365af4e39..00000000000 --- a/go_modules/helpers/go.sum +++ /dev/null @@ -1,2 +0,0 @@ -github.com/Masterminds/vcs v1.13.3 h1:IIA2aBdXvfbIM+yl/eTnL4hb1XwdpvuQLglAix1gweE= -github.com/Masterminds/vcs v1.13.3/go.mod h1:TiE7xuEjl1N4j016moRd6vezp6e6Lz23gypeXfzXeW8= diff --git a/go_modules/helpers/importresolver/main.go b/go_modules/helpers/importresolver/main.go deleted file mode 100644 index 938980edbef..00000000000 --- a/go_modules/helpers/importresolver/main.go +++ /dev/null @@ -1,34 +0,0 @@ -package importresolver - -import ( - "io/ioutil" - "strings" - - "github.com/Masterminds/vcs" -) - -type Args struct { - Import string -} - -func VCSRemoteForImport(args *Args) (interface{}, error) { - remote := args.Import - scheme := strings.Split(remote, ":")[0] - switch scheme { - case "http", "https": - default: - remote = "https://" + remote - } - - local, err := ioutil.TempDir("", "unused-vcs-local-dir") - if err != nil { - return nil, err - } - - repo, err := vcs.NewRepo(remote, local) - if err != nil { - return nil, err - } - - return repo.Remote(), nil -} diff --git a/go_modules/helpers/main.go b/go_modules/helpers/main.go deleted file mode 100644 index 4f8d6e8747d..00000000000 --- a/go_modules/helpers/main.go +++ /dev/null @@ -1,67 +0,0 @@ -package main - -import ( - "encoding/json" - "fmt" - "log" - "os" - - "github.com/dependabot/dependabot-core/go_modules/helpers/importresolver" -) - -type HelperParams struct { - Function string `json:"function"` - Args json.RawMessage `json:"args"` -} - -type Output struct { - Error string `json:"error,omitempty"` - Result interface{} `json:"result,omitempty"` -} - -func main() { - d := json.NewDecoder(os.Stdin) - helperParams := &HelperParams{} - if err := d.Decode(helperParams); err != nil { - abort(err) - } - - var ( - funcOut interface{} - funcErr error - ) - switch helperParams.Function { - case "getVcsRemoteForImport": - var args importresolver.Args - parseArgs(helperParams.Args, &args) - funcOut, funcErr = importresolver.VCSRemoteForImport(&args) - default: - abort(fmt.Errorf("unrecognised function '%s'", helperParams.Function)) - } - - if funcErr != nil { - abort(funcErr) - } - - output(&Output{Result: funcOut}) -} - -func parseArgs(data []byte, args interface{}) { - if err := json.Unmarshal(data, args); err != nil { - abort(err) - } -} - -func output(o *Output) { - bytes, jsonErr := json.Marshal(o) - if jsonErr != nil { - log.Fatal(jsonErr) - } - - os.Stdout.Write(bytes) -} - -func abort(err error) { - output(&Output{Error: err.Error()}) - os.Exit(1) -} diff --git a/go_modules/lib/dependabot/go_modules/metadata_finder.rb b/go_modules/lib/dependabot/go_modules/metadata_finder.rb index 6875265b168..8c4c7589f38 100644 --- a/go_modules/lib/dependabot/go_modules/metadata_finder.rb +++ b/go_modules/lib/dependabot/go_modules/metadata_finder.rb @@ -13,8 +13,7 @@ def look_up_source return look_up_git_dependency_source if git_dependency? path_str = (specified_source_string || dependency.name) - url = Dependabot::GoModules::PathConverter. - git_url_for_path_without_go_helper(path_str) + url = Dependabot::GoModules::PathConverter.git_url_for_path(path_str) Source.from_url(url) if url end diff --git a/go_modules/lib/dependabot/go_modules/path_converter.rb b/go_modules/lib/dependabot/go_modules/path_converter.rb index 1a6646c25b1..7d941a8a208 100644 --- a/go_modules/lib/dependabot/go_modules/path_converter.rb +++ b/go_modules/lib/dependabot/go_modules/path_converter.rb @@ -10,22 +10,8 @@ module Dependabot module GoModules module PathConverter - def self.git_url_for_path(path) - # Save a query by manually converting golang.org/x names - import_path = path.gsub(%r{^golang\.org/x}, "github.com/golang") - - SharedHelpers.run_helper_subprocess( - command: NativeHelpers.helper_path, - function: "getVcsRemoteForImport", - args: { import: import_path } - ) - end - # rubocop:disable Metrics/PerceivedComplexity - # Used in dependabot-backend, which doesn't have access to any Go - # helpers. - # TODO: remove the need for this. - def self.git_url_for_path_without_go_helper(path) + def self.git_url_for_path(path) # Save a query by manually converting golang.org/x names tmp_path = path.gsub(%r{^golang\.org/x}, "github.com/golang") @@ -56,8 +42,8 @@ def self.git_url_for_path_without_go_helper(path) # rubocop:enable Metrics/PerceivedComplexity def self.fetch_path_metadata(path) - # TODO: This is not robust! Instead, we should shell out to Go and - # use https://github.com/Masterminds/vcs. + # TODO: update this comment to explain why it's leveraging the `go-get=1` trick + # and why may not be robust response = Dependabot::RegistryClient.get(url: "https://#{path}?go-get=1") return unless response.status == 200 diff --git a/go_modules/spec/dependabot/go_modules/path_converter_spec.rb b/go_modules/spec/dependabot/go_modules/path_converter_spec.rb index 8f88b012ae4..eb4c906d182 100644 --- a/go_modules/spec/dependabot/go_modules/path_converter_spec.rb +++ b/go_modules/spec/dependabot/go_modules/path_converter_spec.rb @@ -29,6 +29,31 @@ it { is_expected.to eq("https://github.com/kubernetes/apimachinery") } end + # TODO: Below was test case for ruby helper, above is test case for native go helper + # notice the extra fixture stubbing required for Ruby, figure out how robust this is... + # + # context "with a vanity URL that needs to be fetched" do + # let(:path) { "k8s.io/apimachinery" } + + # before do + # stub_request(:get, "https://k8s.io/apimachinery?go-get=1"). + # to_return(status: 200, body: vanity_response) + # end + # let(:vanity_response) do + # fixture("repo_responses", "k8s_io_apimachinery.html") + # end + + # it { is_expected.to eq("https://github.com/kubernetes/apimachinery") } + + # context "and returns a git source hosted with an unknown provider" do + # let(:vanity_response) do + # fixture("go", "repo_responses", "unknown_git_source.html") + + # it { is_expected.to eq("https://sf.com/kubernetes/apimachinery") } + # end + # end + # end + xcontext "with a vanity URL that redirects" do let(:path) { "code.cloudfoundry.org/bytefmt" } it { is_expected.to eq("https://github.com/cloudfoundry/bytefmt") } @@ -44,47 +69,4 @@ it { is_expected.to eq("https://github.com/drewolson/testflight") } end end - - describe ".git_url_for_path_without_go_helper" do - subject { described_class.git_url_for_path_without_go_helper(path) } - - let(:path) { "gopkg.in/guregu/null.v3" } - - context "with a path that is immediately recognisable as a git source" do - let(:path) { "github.com/drewolson/testflight" } - it { is_expected.to eq("https://github.com/drewolson/testflight") } - end - - context "with a golang.org path" do - let(:path) { "golang.org/x/tools" } - it { is_expected.to eq("https://github.com/golang/tools") } - end - - context "with a path that ends in .git" do - let(:path) { "git.fd.io/govpp.git" } - it { is_expected.to eq("https://git.fd.io/govpp.git") } - end - - context "with a vanity URL that needs to be fetched" do - let(:path) { "k8s.io/apimachinery" } - - before do - stub_request(:get, "https://k8s.io/apimachinery?go-get=1"). - to_return(status: 200, body: vanity_response) - end - let(:vanity_response) do - fixture("repo_responses", "k8s_io_apimachinery.html") - end - - it { is_expected.to eq("https://github.com/kubernetes/apimachinery") } - - context "and returns a git source hosted with an unknown provider" do - let(:vanity_response) do - fixture("go", "repo_responses", "unknown_git_source.html") - - it { is_expected.to eq("https://sf.com/kubernetes/apimachinery") } - end - end - end - end end