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.

Sign up for GitHub

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

api: Strict port number handling and --enable-rpc deprecation #6459

Merged
merged 4 commits into from
Sep 14, 2021
Merged

api: Strict port number handling and --enable-rpc deprecation #6459

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
08109c2
Strict port validation and deprecate --enable-rpc
ahmetb Sep 1, 2021
1fd70e2
add warning if --enable-rpc but no ports are set
ahmetb Sep 1, 2021
d81eb12
replace deprecation warning with chosen rpc port
ahmetb Sep 13, 2021
4fb717b
show warning only if http port is also unspecified
ahmetb Sep 13, 2021
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
1 change: 0 additions & 1 deletion cmd/skaffold/app/cmd/debug_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ func TestNewCmdDebug(t *testing.T) {

t.CheckDeepEqual(true, opts.Tail)
t.CheckDeepEqual(false, opts.Force)
t.CheckDeepEqual(true, opts.EnableRPC)
})
}

Expand Down
1 change: 0 additions & 1 deletion cmd/skaffold/app/cmd/dev_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,5 @@ func TestNewCmdDev(t *testing.T) {

t.CheckDeepEqual(true, opts.Tail)
t.CheckDeepEqual(false, opts.Force)
t.CheckDeepEqual(true, opts.EnableRPC)
})
}
30 changes: 14 additions & 16 deletions cmd/skaffold/app/cmd/flags.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import (
"github.com/spf13/pflag"

"github.com/GoogleContainerTools/skaffold/cmd/skaffold/app/flags"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/instrumentation"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/output/log"
)
Expand Down Expand Up @@ -170,17 +169,14 @@ var flagRegistry = []Flag{
DefinedOn: []string{"dev", "build", "run", "debug"},
},
{
Name: "enable-rpc",
Usage: "Enable gRPC for exposing Skaffold events",
Value: &opts.EnableRPC,
DefValue: false,
DefValuePerCommand: map[string]interface{}{
"dev": true,
"debug": true,
},
Name: "enable-rpc",
Usage: "Enable gRPC or HTTP APIs for exposing Skaffold events",
Value: &opts.EnableRPC,
DefValue: false,
FlagAddMethod: "BoolVar",
DefinedOn: []string{"dev", "build", "run", "debug", "deploy", "render", "apply", "test"},
IsEnum: true,
Deprecated: "flags --rpc-port or --rpc-http-port now imply --enable-rpc=true, so please use only those instead",
},
{
Name: "wait-for-connection",
Expand All @@ -193,7 +189,7 @@ var flagRegistry = []Flag{
},
{
Name: "event-log-file",
Usage: "Save Skaffold events to the provided file after skaffold has finished executing, requires --enable-rpc=true",
Usage: "Save Skaffold events to the provided file after skaffold has finished executing, requires --rpc-port or --rpc-http-port",
Hidden: true,
Value: &opts.EventLogFile,
DefValue: "",
Expand All @@ -202,18 +198,18 @@ var flagRegistry = []Flag{
},
{
Name: "rpc-port",
Usage: "tcp port to expose event API",
Usage: "tcp port to expose the Skaffold API over gRPC",
Value: &opts.RPCPort,
DefValue: constants.DefaultRPCPort,
FlagAddMethod: "IntVar",
DefValue: nil,
FlagAddMethod: "Var",
DefinedOn: []string{"dev", "build", "run", "debug", "deploy", "test"},
},
{
Name: "rpc-http-port",
Usage: "tcp port to expose event REST API over HTTP",
Usage: "tcp port to expose the Skaffold API over HTTP REST",
Value: &opts.RPCHTTPPort,
DefValue: constants.DefaultRPCHTTPPort,
FlagAddMethod: "IntVar",
DefValue: nil,
FlagAddMethod: "Var",
DefinedOn: []string{"dev", "build", "run", "debug", "deploy", "test"},
},
{
Expand Down Expand Up @@ -601,6 +597,8 @@ func methodNameByType(v reflect.Value) string {
switch t {
case reflect.Bool:
return "BoolVar"
case reflect.Int:
return "IntVar"
case reflect.String:
return "StringVar"
case reflect.Slice:
Expand Down
2 changes: 1 addition & 1 deletion cmd/skaffold/app/cmd/init_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ func TestFlagsToConfigVersion(t *testing.T) {

// we ignore Skaffold options
test.expectedConfig.Opts = capturedConfig.Opts
t.CheckErrorAndDeepEqual(test.shouldErr, err, test.expectedConfig, capturedConfig, cmp.AllowUnexported(cfg.StringOrUndefined{}, cfg.BoolOrUndefined{}, cfg.SyncRemoteCacheOption{}))
t.CheckErrorAndDeepEqual(test.shouldErr, err, test.expectedConfig, capturedConfig, cmp.AllowUnexported(cfg.StringOrUndefined{}, cfg.BoolOrUndefined{}, cfg.IntOrUndefined{}, cfg.SyncRemoteCacheOption{}))
})
}
}
40 changes: 19 additions & 21 deletions docs/content/en/docs/design/api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,38 +21,36 @@ To retrieve information about the Skaffold pipeline, the Skaffold API provides t
To control the individual phases of the Skaffold, the Skaffold API provides [fine-grained control]({{< relref "#control-api" >}})
over the individual phases of the pipeline (build, deploy, and sync).


## Connecting to the Skaffold API
The Skaffold API is `gRPC` based, and it is also exposed via the gRPC gateway as a JSON over HTTP service.
The server is hosted locally on the same host where the skaffold process is running, and will serve by default on ports 50051 and 50052.
These ports can be configured through the `--rpc-port` and `--rpc-http-port` flags.

For reference, we generate the server's [gRPC service definitions and message protos]({{< relref "/docs/references/api/grpc" >}}) as well as the [Swagger based HTTP API Spec]({{< relref "/docs/references/api/swagger" >}}).
The API can be enabled via setting the `--rpc-port` or `--rpc-http-port` flags (or both)
depending on whether you want to enable the gRPC API or the HTTP REST API, respectively.


### HTTP server
The HTTP API is exposed on port `50052` by default. The default HTTP port can be overridden with the `--rpc-http-port` flag.
If the HTTP API port is taken, Skaffold will find the next available port.
The final port can be found from Skaffold's startup logs.
{{< alert title="Note">}}
The `--enable-rpc` flag is now deprecated in favor of `--rpc-port` and `--rpc-http-port` flags.
{{</alert>}}

```code
$ skaffold dev
WARN[0000] port 50052 for gRPC HTTP server already in use: using 50055 instead
```

### gRPC Server
For reference, we generate the server's [gRPC service definitions and message protos]({{< relref "/docs/references/api/grpc" >}}) as well as the [Swagger based HTTP API Spec]({{< relref "/docs/references/api/swagger" >}}).

## gRPC Server

The gRPC API is exposed on port `50051` by default and can be overridden with the `--rpc-port` flag.
As with the HTTP API, if this port is taken, Skaffold will find the next available port.
You can find this port from Skaffold's logs on startup.
The gRPC API can be started by specifying the `--rpc-port` flag. If the specified port is not available, Skaffold will
exit with failure.

```code
$ skaffold dev
WARN[0000] port 50051 for gRPC server already in use: using 50053 instead
```
### HTTP server

The HTTP REST API can be started by specifying the `--rpc-http-port` flag. If the specified port is not available,
Skaffold will exit with failure.

Starting the HTTP REST API will also start the gRPC API as it proxies the requests to the gRPC API. By default, Skaffold
chooses a random available port for gRPC, but it can be customized (see below).

#### Creating a gRPC Client
To connect to the `gRPC` server at default port `50051`, create a client using the following code snippet.

To connect to the `gRPC` server at the specified port, create a client using the following code snippet.

{{< alert title="Note" >}}
The skaffold gRPC server is not compatible with HTTPS, so connections need to be marked as insecure with `grpc.WithInsecure()`
Expand Down
Loading