From f26ff28ad44cab1327a4dde4cfc3bcfd7410e8cf Mon Sep 17 00:00:00 2001 From: sophia Date: Tue, 6 Sep 2022 12:53:28 -0400 Subject: [PATCH 01/11] Set provider if available --- internal/core/vagrantfile.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/internal/core/vagrantfile.go b/internal/core/vagrantfile.go index abd6f2795..c2c9251e8 100644 --- a/internal/core/vagrantfile.go +++ b/internal/core/vagrantfile.go @@ -456,6 +456,7 @@ func (v *Vagrantfile) Target( Project: v.targetSource, }, ), + WithProvider(provider), } var vf *Vagrantfile @@ -468,12 +469,15 @@ func (v *Vagrantfile) Target( if err != nil { return nil, err } + rawTarget := target.(*Target) + if provider != "" { + rawTarget.target.Provider = provider + } // Since the target config gives us a Vagrantfile which is // attached to the project, we need to clone it and attach // it to the target we loaded if vf != nil { - rawTarget := target.(*Target) tvf := vf.clone(name) if err = tvf.Init(); err != nil { From 571daea639cfd30e6c561e526fe1a74437421139 Mon Sep 17 00:00:00 2001 From: sophia Date: Tue, 6 Sep 2022 15:38:03 -0400 Subject: [PATCH 02/11] Validate provider by default when collecting machine config --- lib/vagrant/environment.rb | 2 +- lib/vagrant/vagrantfile/remote.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/vagrant/environment.rb b/lib/vagrant/environment.rb index de99cbf3b..aec7af05e 100644 --- a/lib/vagrant/environment.rb +++ b/lib/vagrant/environment.rb @@ -351,7 +351,7 @@ module Vagrant # then look there. root_config = vagrantfile.config if opts[:machine] - machine_info = vagrantfile.machine_config(opts[:machine], nil, nil, nil) + machine_info = vagrantfile.machine_config(opts[:machine]) root_config = machine_info[:config] end diff --git a/lib/vagrant/vagrantfile/remote.rb b/lib/vagrant/vagrantfile/remote.rb index ab0ff322b..9b2f2bb7e 100644 --- a/lib/vagrant/vagrantfile/remote.rb +++ b/lib/vagrant/vagrantfile/remote.rb @@ -25,7 +25,7 @@ module Vagrant client.target_names end - def machine_config(name, provider, _, _, validate_provider) + def machine_config(name, provider, _, _, validate_provider=true) client.machine_config(name, provider, validate_provider) end end From 6d4f769c8745074e07540225cb4d90b74a6fd1cc Mon Sep 17 00:00:00 2001 From: sophia Date: Tue, 6 Sep 2022 17:54:28 -0400 Subject: [PATCH 03/11] Validate provider --- internal/core/vagrantfile.go | 28 +++++++++++++++++-- .../serve/service/provider_service.rb | 2 +- 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/internal/core/vagrantfile.go b/internal/core/vagrantfile.go index c2c9251e8..9fc8eb935 100644 --- a/internal/core/vagrantfile.go +++ b/internal/core/vagrantfile.go @@ -7,6 +7,7 @@ import ( "github.com/hashicorp/go-argmapper" "github.com/hashicorp/go-hclog" + "github.com/pkg/errors" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" "google.golang.org/protobuf/proto" @@ -18,6 +19,7 @@ import ( "github.com/hashicorp/vagrant-plugin-sdk/internal-shared/cleanup" "github.com/hashicorp/vagrant-plugin-sdk/internal-shared/dynamic" "github.com/hashicorp/vagrant-plugin-sdk/internal-shared/protomappers" + "github.com/hashicorp/vagrant-plugin-sdk/localizer" "github.com/hashicorp/vagrant-plugin-sdk/proto/vagrant_plugin_sdk" "github.com/hashicorp/vagrant/internal/plugin" "github.com/hashicorp/vagrant/internal/server/proto/vagrant_server" @@ -444,7 +446,7 @@ func (v *Vagrantfile) Target( return nil, err } - conf, err := v.TargetConfig(name, provider, false) + conf, err := v.TargetConfig(name, provider, true) if err != nil { return } @@ -496,7 +498,6 @@ func (v *Vagrantfile) Target( // Generate a new Vagrantfile for the given target // NOTE: This function may return a nil result without an error -// TODO(spox): Provider validation is not currently implemented // TODO(spox): Needs box configuration applied func (v *Vagrantfile) TargetConfig( name, // name of the target @@ -511,6 +512,29 @@ func (v *Vagrantfile) TargetConfig( return nil, err } + if provider != "" { + pp, err := v.factory.plugins.Find(provider, component.ProviderType) + if err != nil { + return nil, err + } + if validateProvider { + usable, err := pp.Component.(core.Provider).Usable() + if err != nil { + return nil, err + } + if !usable { + // TODO: include message provided in the error + return nil, errors.New( + localizer.LocalizeMsg( + "provider_not_usable", + map[string]string{"Provider": provider, "Machine": name}, + ), + ) + + } + } + } + cid := name + "+" + provider if cv := v.cache.Get(cid); cv != nil { return cv.(core.Vagrantfile), nil diff --git a/plugins/commands/serve/service/provider_service.rb b/plugins/commands/serve/service/provider_service.rb index e978645ca..cf66efe38 100644 --- a/plugins/commands/serve/service/provider_service.rb +++ b/plugins/commands/serve/service/provider_service.rb @@ -20,7 +20,7 @@ module VagrantPlugins def usable(req, ctx) with_plugin(ctx, :providers, broker: broker) do |plugin| - is_usable = plugin.usable? + is_usable = plugin.usable?(true) SDK::Provider::UsableResp.new( is_usable: is_usable, ) From add66c4c2a39a76bc15d20780633f5fb2e604aea Mon Sep 17 00:00:00 2001 From: sophia Date: Thu, 8 Sep 2022 13:02:49 -0400 Subject: [PATCH 04/11] Include message in rpc status error from ruby --- plugins/commands/serve/util/exception_transformer.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plugins/commands/serve/util/exception_transformer.rb b/plugins/commands/serve/util/exception_transformer.rb index d8c4779af..dc12343b7 100644 --- a/plugins/commands/serve/util/exception_transformer.rb +++ b/plugins/commands/serve/util/exception_transformer.rb @@ -62,7 +62,8 @@ module VagrantPlugins ) proto = Google::Rpc::Status.new( code: GRPC::Core::StatusCodes::UNKNOWN, - details: [localized_msg_details_any] + details: [localized_msg_details_any], + message: message, ) metadata[GRPC_DETAILS_METADATA_KEY] = Google::Rpc::Status.encode(proto) end From ccb89c01438bf094197ce2a5f5f6aece7711ffe4 Mon Sep 17 00:00:00 2001 From: sophia Date: Thu, 8 Sep 2022 18:21:21 -0400 Subject: [PATCH 05/11] Don't check for errors when checking if a provider is usable If a provider is unusable then Usable() will throw an error. When checking for a default provider, Vagrant should not raise an error if there is an issue with executing Usable(). --- internal/core/project.go | 5 +---- lib/vagrant/environment.rb | 2 +- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/internal/core/project.go b/internal/core/project.go index ae7a1194d..7b557d98e 100644 --- a/internal/core/project.go +++ b/internal/core/project.go @@ -392,10 +392,7 @@ func (p *Project) DefaultProvider(opts *core.DefaultProviderOptions) (string, er if opts.CheckUsable { logger.Debug("Checking usable on provider", "provider", pp.Name) pluginImpl := plug.Plugin.(core.Provider) - usable, err := pluginImpl.Usable() - if err != nil { - return "", err - } + usable, _ := pluginImpl.Usable() if !usable { logger.Debug("Skipping unusable provider", "provider", pp.Name) continue diff --git a/lib/vagrant/environment.rb b/lib/vagrant/environment.rb index aec7af05e..de99cbf3b 100644 --- a/lib/vagrant/environment.rb +++ b/lib/vagrant/environment.rb @@ -351,7 +351,7 @@ module Vagrant # then look there. root_config = vagrantfile.config if opts[:machine] - machine_info = vagrantfile.machine_config(opts[:machine]) + machine_info = vagrantfile.machine_config(opts[:machine], nil, nil, nil) root_config = machine_info[:config] end From 880db9adc0a454a1c72a988099547f12ad42700d Mon Sep 17 00:00:00 2001 From: sophia Date: Fri, 9 Sep 2022 13:07:55 -0400 Subject: [PATCH 06/11] Add dependency on google rpc protos --- go.mod | 1 + go.sum | 2 ++ 2 files changed, 3 insertions(+) diff --git a/go.mod b/go.mod index 0ca78dd4d..f3092d1ca 100644 --- a/go.mod +++ b/go.mod @@ -94,6 +94,7 @@ require ( github.com/go-git/gcfg v1.5.0 // indirect github.com/go-git/go-billy/v5 v5.0.0 // indirect github.com/go-test/deep v1.0.7 // indirect + github.com/gogo/googleapis v1.4.1 // indirect github.com/gogo/protobuf v1.3.2 // indirect github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect github.com/golang/protobuf v1.5.2 // indirect diff --git a/go.sum b/go.sum index 0ce5451f9..fb1d6f208 100644 --- a/go.sum +++ b/go.sum @@ -209,6 +209,8 @@ github.com/go-test/deep v1.0.7 h1:/VSMRlnY/JSyqxQUzQLKVMAskpY/NZKFA5j2P+0pP2M= github.com/go-test/deep v1.0.7/go.mod h1:QV8Hv/iy04NyLBxAdO9njL0iVPN1S4d/A3NVv1V36o8= github.com/gofrs/flock v0.8.0 h1:MSdYClljsF3PbENUUEx85nkWfJSGfzYI9yEBZOJz6CY= github.com/gofrs/flock v0.8.0/go.mod h1:F1TvTiK9OcQqauNUHlbJvyl9Qa1QvF/gOUDKA14jxHU= +github.com/gogo/googleapis v1.4.1 h1:1Yx4Myt7BxzvUr5ldGSbwYiZG6t9wGBZ+8/fX3Wvtq0= +github.com/gogo/googleapis v1.4.1/go.mod h1:2lpHqI5OcWCtVElxXnPt+s8oJvMpySlOyM6xDCrzib4= github.com/gogo/protobuf v1.3.1/go.mod h1:SlYgWuQ5SjCEi6WLHjHCa1yvBfUnHcTbrrZtXPKa29o= github.com/gogo/protobuf v1.3.2 h1:Ov1cvc58UF3b5XjBnZv7+opcTcQFZebYjWzi34vdm4Q= github.com/gogo/protobuf v1.3.2/go.mod h1:P1XiOD3dCwIKUDQYPy72D8LYyHL2YPYrpS2s69NZV8Q= From 9a276d7818ef0da8de21c13c592864a780702fbd Mon Sep 17 00:00:00 2001 From: sophia Date: Fri, 9 Sep 2022 13:08:23 -0400 Subject: [PATCH 07/11] Get additional message into error --- internal/core/vagrantfile.go | 46 +++++++++++++++++++++++++++--------- 1 file changed, 35 insertions(+), 11 deletions(-) diff --git a/internal/core/vagrantfile.go b/internal/core/vagrantfile.go index 9fc8eb935..54d25394a 100644 --- a/internal/core/vagrantfile.go +++ b/internal/core/vagrantfile.go @@ -5,11 +5,13 @@ import ( "fmt" "sync" + "github.com/gogo/googleapis/google/rpc" + "github.com/golang/protobuf/ptypes" "github.com/hashicorp/go-argmapper" "github.com/hashicorp/go-hclog" - "github.com/pkg/errors" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" + "google.golang.org/protobuf/proto" "github.com/hashicorp/vagrant-plugin-sdk/component" @@ -519,19 +521,41 @@ func (v *Vagrantfile) TargetConfig( } if validateProvider { usable, err := pp.Component.(core.Provider).Usable() + if !usable { + errStatus, ok := status.FromError(err) + if !ok { + return nil, err + } + msg := localizer.LocalizeMsg( + "provider_not_usable", + map[string]string{"Provider": provider, "Machine": name}, + ) + // st := status.New(codes.Unknown, msg) + + // protoMsg := &wrapperspb.StringValue{ + // Value: msg, + // } + localizedProtoMsg := &rpc.LocalizedMessage{ + Message: msg, + Locale: "en-US", + } + protoMsgAny, _ := ptypes.MarshalAny(localizedProtoMsg) + // st, _ = st.WithDetails(protoMsg) + // for _, d := range errStatus.Details() { + // st, _ = st.WithDetails(d.(proto.Message)) + // } + errStatusProto := errStatus.Proto() + errStatusProto.Message = msg + errStatusProto.Details = append(errStatusProto.Details, protoMsgAny) + // errStatusProto.Details = []*anypb.Any{protoMsgAny} + + // TODO: include message provided in the error + errStatusP := status.FromProto(errStatusProto) + return nil, errStatusP.Err() + } if err != nil { return nil, err } - if !usable { - // TODO: include message provided in the error - return nil, errors.New( - localizer.LocalizeMsg( - "provider_not_usable", - map[string]string{"Provider": provider, "Machine": name}, - ), - ) - - } } } From bd801af98405155effe9d17156fa063f6e238f5a Mon Sep 17 00:00:00 2001 From: sophia Date: Fri, 9 Sep 2022 14:50:07 -0400 Subject: [PATCH 08/11] Add details to errors --- internal/core/vagrantfile.go | 31 +++++++++---------------------- 1 file changed, 9 insertions(+), 22 deletions(-) diff --git a/internal/core/vagrantfile.go b/internal/core/vagrantfile.go index 54d25394a..7758b4c60 100644 --- a/internal/core/vagrantfile.go +++ b/internal/core/vagrantfile.go @@ -5,10 +5,9 @@ import ( "fmt" "sync" - "github.com/gogo/googleapis/google/rpc" - "github.com/golang/protobuf/ptypes" "github.com/hashicorp/go-argmapper" "github.com/hashicorp/go-hclog" + "google.golang.org/genproto/googleapis/rpc/errdetails" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" @@ -530,28 +529,16 @@ func (v *Vagrantfile) TargetConfig( "provider_not_usable", map[string]string{"Provider": provider, "Machine": name}, ) - // st := status.New(codes.Unknown, msg) - - // protoMsg := &wrapperspb.StringValue{ - // Value: msg, - // } - localizedProtoMsg := &rpc.LocalizedMessage{ + localizedProtoMsg := &errdetails.LocalizedMessage{ Message: msg, - Locale: "en-US", + Locale: "en", } - protoMsgAny, _ := ptypes.MarshalAny(localizedProtoMsg) - // st, _ = st.WithDetails(protoMsg) - // for _, d := range errStatus.Details() { - // st, _ = st.WithDetails(d.(proto.Message)) - // } - errStatusProto := errStatus.Proto() - errStatusProto.Message = msg - errStatusProto.Details = append(errStatusProto.Details, protoMsgAny) - // errStatusProto.Details = []*anypb.Any{protoMsgAny} - - // TODO: include message provided in the error - errStatusP := status.FromProto(errStatusProto) - return nil, errStatusP.Err() + returnErr := status.New(errStatus.Code(), msg) + returnErr, _ = returnErr.WithDetails(localizedProtoMsg) + for _, d := range errStatus.Details() { + returnErr, _ = returnErr.WithDetails(d.(*errdetails.LocalizedMessage)) + } + return nil, returnErr.Err() } if err != nil { return nil, err From b9fb6c5f5f1c4e07d50eb8e2a1faca6f1ceafcd4 Mon Sep 17 00:00:00 2001 From: sophia Date: Fri, 9 Sep 2022 15:51:30 -0400 Subject: [PATCH 09/11] Refactor to use localizer to create error with status --- internal/core/vagrantfile.go | 24 ++++++------------------ 1 file changed, 6 insertions(+), 18 deletions(-) diff --git a/internal/core/vagrantfile.go b/internal/core/vagrantfile.go index 7758b4c60..67237cda1 100644 --- a/internal/core/vagrantfile.go +++ b/internal/core/vagrantfile.go @@ -7,7 +7,6 @@ import ( "github.com/hashicorp/go-argmapper" "github.com/hashicorp/go-hclog" - "google.golang.org/genproto/googleapis/rpc/errdetails" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" @@ -521,24 +520,13 @@ func (v *Vagrantfile) TargetConfig( if validateProvider { usable, err := pp.Component.(core.Provider).Usable() if !usable { - errStatus, ok := status.FromError(err) - if !ok { - return nil, err + if errStatus, ok := status.FromError(err); ok { + return nil, localizer.LocalizeStatusErr( + "provider_not_usable", + map[string]string{"Provider": provider, "Machine": name}, + errStatus, + ) } - msg := localizer.LocalizeMsg( - "provider_not_usable", - map[string]string{"Provider": provider, "Machine": name}, - ) - localizedProtoMsg := &errdetails.LocalizedMessage{ - Message: msg, - Locale: "en", - } - returnErr := status.New(errStatus.Code(), msg) - returnErr, _ = returnErr.WithDetails(localizedProtoMsg) - for _, d := range errStatus.Details() { - returnErr, _ = returnErr.WithDetails(d.(*errdetails.LocalizedMessage)) - } - return nil, returnErr.Err() } if err != nil { return nil, err From 49280286ad225a9c97be3beee4d05072b1c11d65 Mon Sep 17 00:00:00 2001 From: sophia Date: Wed, 14 Sep 2022 15:12:32 -0400 Subject: [PATCH 10/11] Strip new lines from error details The error details message should just be one string without spacing so the terminal can set a output length. This will allow messages from Ruby and Go errors to have similar formating. --- internal/core/vagrantfile.go | 1 + plugins/commands/serve/util/exception_transformer.rb | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/core/vagrantfile.go b/internal/core/vagrantfile.go index 67237cda1..fd786155c 100644 --- a/internal/core/vagrantfile.go +++ b/internal/core/vagrantfile.go @@ -525,6 +525,7 @@ func (v *Vagrantfile) TargetConfig( "provider_not_usable", map[string]string{"Provider": provider, "Machine": name}, errStatus, + true, ) } } diff --git a/plugins/commands/serve/util/exception_transformer.rb b/plugins/commands/serve/util/exception_transformer.rb index dc12343b7..bf5c61239 100644 --- a/plugins/commands/serve/util/exception_transformer.rb +++ b/plugins/commands/serve/util/exception_transformer.rb @@ -58,7 +58,7 @@ module VagrantPlugins if err.is_a? Vagrant::Errors::VagrantError localized_msg_details_any = Google::Protobuf::Any.new localized_msg_details_any.pack( - Google::Rpc::LocalizedMessage.new(locale: "en-US", message: message) + Google::Rpc::LocalizedMessage.new(locale: "en-US", message: message.gsub("\n", " ")) ) proto = Google::Rpc::Status.new( code: GRPC::Core::StatusCodes::UNKNOWN, From 33f0267cc4cf37f87602e82a1e6a2f7d75358745 Mon Sep 17 00:00:00 2001 From: sophia Date: Mon, 19 Sep 2022 15:15:08 -0400 Subject: [PATCH 11/11] Bump sdk --- go.mod | 2 +- go.sum | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/go.mod b/go.mod index f3092d1ca..7ba13728a 100644 --- a/go.mod +++ b/go.mod @@ -25,7 +25,7 @@ require ( github.com/hashicorp/go-version v1.3.0 github.com/hashicorp/hcl/v2 v2.11.1 github.com/hashicorp/nomad/api v0.0.0-20200814140818-42de70466a9d - github.com/hashicorp/vagrant-plugin-sdk v0.0.0-20220919180525-50c632cd450d + github.com/hashicorp/vagrant-plugin-sdk v0.0.0-20220919180735-d47bfe003e94 github.com/imdario/mergo v0.3.11 github.com/improbable-eng/grpc-web v0.13.0 github.com/kr/text v0.2.0 diff --git a/go.sum b/go.sum index fb1d6f208..12bace98d 100644 --- a/go.sum +++ b/go.sum @@ -365,6 +365,8 @@ github.com/hashicorp/vagrant-plugin-sdk v0.0.0-20220913204040-793a2626f6f9 h1:wf github.com/hashicorp/vagrant-plugin-sdk v0.0.0-20220913204040-793a2626f6f9/go.mod h1:zA5vDskG3gH306C+obL+yURiUiLMAlx52yqO8MC2r9w= github.com/hashicorp/vagrant-plugin-sdk v0.0.0-20220919180525-50c632cd450d h1:TqCLroDhxzGMXE7LrgqDayOku2oRJ4vjROX7ghpzqsI= github.com/hashicorp/vagrant-plugin-sdk v0.0.0-20220919180525-50c632cd450d/go.mod h1:zA5vDskG3gH306C+obL+yURiUiLMAlx52yqO8MC2r9w= +github.com/hashicorp/vagrant-plugin-sdk v0.0.0-20220919180735-d47bfe003e94 h1:CGq9dOg/kK0ihxx81H59xEHGfTvRs0ls8qCL3Bujdgo= +github.com/hashicorp/vagrant-plugin-sdk v0.0.0-20220919180735-d47bfe003e94/go.mod h1:zA5vDskG3gH306C+obL+yURiUiLMAlx52yqO8MC2r9w= github.com/hashicorp/yamux v0.0.0-20180604194846-3520598351bb/go.mod h1:+NfK9FKeTrX5uv1uIXGdwYDTeHna2qgaIlx54MXqjAM= github.com/hashicorp/yamux v0.0.0-20200609203250-aecfd211c9ce h1:7UnVY3T/ZnHUrfviiAgIUjg2PXxsQfs5bphsG8F7Keo= github.com/hashicorp/yamux v0.0.0-20200609203250-aecfd211c9ce/go.mod h1:+NfK9FKeTrX5uv1uIXGdwYDTeHna2qgaIlx54MXqjAM=