Add org scoped port forwarding + fix test formatting (#4497)

* Add org scoped port forwarding + fix test formatting

* Redesign port visibility

* Update pkg/cmd/codespace/ports.go

Co-authored-by: Jose Garcia <josebalius@github.com>

* Change sub command to privacy

* Example pr comment

* Fix test mock

* Fix test mock

Co-authored-by: Jose Garcia <josebalius@github.com>
This commit is contained in:
Marwan Sulaiman 2021-10-13 13:56:03 -04:00 committed by GitHub
parent a033b85fa2
commit b9bdef2b00
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 73 additions and 72 deletions

View file

@ -100,7 +100,7 @@ func Test_CurrentBranch(t *testing.T) {
result, err := CurrentBranch()
if err != nil {
t.Errorf("got unexpected error: %w", err)
t.Errorf("got unexpected error: %v", err)
}
if result != v.Expected {
t.Errorf("unexpected branch name: %s instead of %s", result, v.Expected)

View file

@ -40,9 +40,8 @@ func newPortsCmd(app *App) *cobra.Command {
portsCmd.PersistentFlags().StringVarP(&codespace, "codespace", "c", "", "Name of the codespace")
portsCmd.Flags().BoolVar(&asJSON, "json", false, "Output as JSON")
portsCmd.AddCommand(newPortsPublicCmd(app))
portsCmd.AddCommand(newPortsPrivateCmd(app))
portsCmd.AddCommand(newPortsForwardCmd(app))
portsCmd.AddCommand(newPortsPrivacyCmd(app))
return portsCmd
}
@ -79,7 +78,7 @@ func (a *App) ListPorts(ctx context.Context, codespaceName string, asJSON bool)
}
table := output.NewTable(os.Stdout, asJSON)
table.SetHeader([]string{"Label", "Port", "Public", "Browse URL"})
table.SetHeader([]string{"Label", "Port", "Privacy", "Browse URL"})
for _, port := range ports {
sourcePort := strconv.Itoa(port.SourcePort)
var portName string
@ -92,7 +91,7 @@ func (a *App) ListPorts(ctx context.Context, codespaceName string, asJSON bool)
table.Append([]string{
portName,
sourcePort,
strings.ToUpper(strconv.FormatBool(port.IsPublic)),
port.Privacy,
fmt.Sprintf("https://%s-%s.githubpreview.dev/", codespace.Name, sourcePort),
})
}
@ -145,13 +144,16 @@ func getDevContainer(ctx context.Context, apiClient apiClient, codespace *api.Co
return ch
}
// newPortsPublicCmd returns a Cobra "ports public" subcommand, which makes a given port public.
func newPortsPublicCmd(app *App) *cobra.Command {
func newPortsPrivacyCmd(app *App) *cobra.Command {
return &cobra.Command{
Use: "public <port>",
Short: "Mark port as public",
Args: cobra.ExactArgs(1),
Use: "privacy <port:public|private|org>...",
Short: "Change the privacy of the forwarded port",
Example: "gh codespace ports privacy 80:org 3000:private 8000:public",
Args: cobra.ArbitraryArgs,
RunE: func(cmd *cobra.Command, args []string) error {
if len(args) == 0 {
return fmt.Errorf("at least one port privacy argument is required")
}
codespace, err := cmd.Flags().GetString("codespace")
if err != nil {
// should only happen if flag is not defined
@ -159,33 +161,16 @@ func newPortsPublicCmd(app *App) *cobra.Command {
// since it's a persistent flag that we control it should never happen
return fmt.Errorf("get codespace flag: %w", err)
}
return app.UpdatePortVisibility(cmd.Context(), codespace, args[0], true)
return app.UpdatePortPrivacy(cmd.Context(), codespace, args)
},
}
}
// newPortsPrivateCmd returns a Cobra "ports private" subcommand, which makes a given port private.
func newPortsPrivateCmd(app *App) *cobra.Command {
return &cobra.Command{
Use: "private <port>",
Short: "Mark port as private",
Args: cobra.ExactArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
codespace, err := cmd.Flags().GetString("codespace")
if err != nil {
// should only happen if flag is not defined
// or if the flag is not of string type
// since it's a persistent flag that we control it should never happen
return fmt.Errorf("get codespace flag: %w", err)
}
return app.UpdatePortVisibility(cmd.Context(), codespace, args[0], false)
},
func (a *App) UpdatePortPrivacy(ctx context.Context, codespaceName string, args []string) (err error) {
ports, err := a.parsePortPrivacies(args)
if err != nil {
return fmt.Errorf("error parsing port arguments: %w", err)
}
}
func (a *App) UpdatePortVisibility(ctx context.Context, codespaceName, sourcePort string, public bool) (err error) {
codespace, err := getOrChooseCodespace(ctx, a.apiClient, codespaceName)
if err != nil {
if err == errNoCodespaces {
@ -200,24 +185,39 @@ func (a *App) UpdatePortVisibility(ctx context.Context, codespaceName, sourcePor
}
defer safeClose(session, &err)
port, err := strconv.Atoi(sourcePort)
if err != nil {
return fmt.Errorf("error reading port number: %w", err)
}
for _, port := range ports {
if err := session.UpdateSharedServerPrivacy(ctx, port.number, port.privacy); err != nil {
return fmt.Errorf("error update port to public: %w", err)
}
if err := session.UpdateSharedVisibility(ctx, port, public); err != nil {
return fmt.Errorf("error update port to public: %w", err)
a.logger.Printf("Port %d is now %s scoped.\n", port.number, port.privacy)
}
state := "PUBLIC"
if !public {
state = "PRIVATE"
}
a.logger.Printf("Port %s is now %s.\n", sourcePort, state)
return nil
}
type portPrivacy struct {
number int
privacy string
}
func (a *App) parsePortPrivacies(args []string) ([]portPrivacy, error) {
ports := make([]portPrivacy, 0, len(args))
for _, a := range args {
fields := strings.Split(a, ":")
if len(fields) != 2 {
return nil, fmt.Errorf("invalid port privacy format for %q", a)
}
portStr, privacy := fields[0], fields[1]
portNumber, err := strconv.Atoi(portStr)
if err != nil {
return nil, fmt.Errorf("invalid port number: %w", err)
}
ports = append(ports, portPrivacy{portNumber, privacy})
}
return ports, nil
}
// NewPortsForwardCmd returns a Cobra "ports forward" subcommand, which forwards a set of
// port pairs from the codespace to localhost.
func newPortsForwardCmd(app *App) *cobra.Command {

View file

@ -48,7 +48,7 @@ func TestConnect(t *testing.T) {
livesharetest.WithRelaySAS(opts.RelaySAS),
)
if err != nil {
t.Errorf("error creating Live Share server: %w", err)
t.Errorf("error creating Live Share server: %v", err)
}
defer server.Close()
opts.RelayEndpoint = "sb" + strings.TrimPrefix(server.URL(), "https")
@ -65,10 +65,10 @@ func TestConnect(t *testing.T) {
select {
case err := <-server.Err():
t.Errorf("error from server: %w", err)
t.Errorf("error from server: %v", err)
case err := <-done:
if err != nil {
t.Errorf("error from client: %w", err)
t.Errorf("error from client: %v", err)
}
}
}

View file

@ -17,7 +17,7 @@ import (
func TestNewPortForwarder(t *testing.T) {
testServer, session, err := makeMockSession()
if err != nil {
t.Errorf("create mock client: %w", err)
t.Errorf("create mock client: %v", err)
}
defer testServer.Close()
pf := NewPortForwarder(session, "ssh", 80, false)
@ -42,7 +42,7 @@ func TestPortForwarderStart(t *testing.T) {
livesharetest.WithStream("stream-id", stream),
)
if err != nil {
t.Errorf("create mock session: %w", err)
t.Errorf("create mock session: %v", err)
}
defer testServer.Close()
@ -86,10 +86,10 @@ func TestPortForwarderStart(t *testing.T) {
select {
case err := <-testServer.Err():
t.Errorf("error from server: %w", err)
t.Errorf("error from server: %v", err)
case err := <-done:
if err != nil {
t.Errorf("error from client: %w", err)
t.Errorf("error from client: %v", err)
}
}
}

View file

@ -41,6 +41,7 @@ type Port struct {
IsPublic bool `json:"isPublic"`
IsTCPServerConnectionEstablished bool `json:"isTCPServerConnectionEstablished"`
HasTLSHandshakePassed bool `json:"hasTLSHandshakePassed"`
Privacy string `json:"privacy"`
}
// startSharing tells the Live Share host to start sharing the specified port from the container.
@ -67,10 +68,10 @@ func (s *Session) GetSharedServers(ctx context.Context) ([]*Port, error) {
return response, nil
}
// UpdateSharedVisibility controls port permissions and whether it can be accessed publicly
// via the Browse URL
func (s *Session) UpdateSharedVisibility(ctx context.Context, port int, public bool) error {
if err := s.rpc.do(ctx, "serverSharing.updateSharedServerVisibility", []interface{}{port, public}, nil); err != nil {
// UpdateSharedServerPrivacy controls port permissions and visibility scopes for who can access its URLs
// in the browser.
func (s *Session) UpdateSharedServerPrivacy(ctx context.Context, port int, visibility string) error {
if err := s.rpc.do(ctx, "serverSharing.updateSharedServerPrivacy", []interface{}{port, visibility}, nil); err != nil {
return err
}

View file

@ -82,7 +82,7 @@ func TestServerStartSharing(t *testing.T) {
defer testServer.Close() //nolint:staticcheck // httptest.Server does not return errors on Close()
if err != nil {
t.Errorf("error creating mock session: %w", err)
t.Errorf("error creating mock session: %v", err)
}
ctx := context.Background()
@ -100,10 +100,10 @@ func TestServerStartSharing(t *testing.T) {
select {
case err := <-testServer.Err():
t.Errorf("error from server: %w", err)
t.Errorf("error from server: %v", err)
case err := <-done:
if err != nil {
t.Errorf("error from client: %w", err)
t.Errorf("error from client: %v", err)
}
}
}
@ -121,7 +121,7 @@ func TestServerGetSharedServers(t *testing.T) {
livesharetest.WithService("serverSharing.getSharedServers", getSharedServers),
)
if err != nil {
t.Errorf("error creating mock session: %w", err)
t.Errorf("error creating mock session: %v", err)
}
defer testServer.Close()
ctx := context.Background()
@ -148,15 +148,15 @@ func TestServerGetSharedServers(t *testing.T) {
select {
case err := <-testServer.Err():
t.Errorf("error from server: %w", err)
t.Errorf("error from server: %v", err)
case err := <-done:
if err != nil {
t.Errorf("error from client: %w", err)
t.Errorf("error from client: %v", err)
}
}
}
func TestServerUpdateSharedVisibility(t *testing.T) {
func TestServerUpdateSharedServerPrivacy(t *testing.T) {
updateSharedVisibility := func(rpcReq *jsonrpc2.Request) (interface{}, error) {
var req []interface{}
if err := json.Unmarshal(*rpcReq.Params, &req); err != nil {
@ -172,33 +172,33 @@ func TestServerUpdateSharedVisibility(t *testing.T) {
} else {
return nil, errors.New("port param is not a float64")
}
if public, ok := req[1].(bool); ok {
if public != true {
return nil, errors.New("pulic param is not expected value")
if privacy, ok := req[1].(string); ok {
if privacy != "public" {
return nil, fmt.Errorf("expected privacy param to be public but got %q", privacy)
}
} else {
return nil, errors.New("public param is not a bool")
return nil, fmt.Errorf("expected privacy param to be a bool but go %T", req[1])
}
return nil, nil
}
testServer, session, err := makeMockSession(
livesharetest.WithService("serverSharing.updateSharedServerVisibility", updateSharedVisibility),
livesharetest.WithService("serverSharing.updateSharedServerPrivacy", updateSharedVisibility),
)
if err != nil {
t.Errorf("creating mock session: %w", err)
t.Errorf("creating mock session: %v", err)
}
defer testServer.Close()
ctx := context.Background()
done := make(chan error)
go func() {
done <- session.UpdateSharedVisibility(ctx, 80, true)
done <- session.UpdateSharedServerPrivacy(ctx, 80, "public")
}()
select {
case err := <-testServer.Err():
t.Errorf("error from server: %w", err)
t.Errorf("error from server: %v", err)
case err := <-done:
if err != nil {
t.Errorf("error from client: %w", err)
t.Errorf("error from client: %v", err)
}
}
}
@ -214,7 +214,7 @@ func TestInvalidHostKey(t *testing.T) {
}
testServer, err := livesharetest.NewServer(opts...)
if err != nil {
t.Errorf("error creating server: %w", err)
t.Errorf("error creating server: %v", err)
}
_, err = Connect(context.Background(), Options{
SessionID: "session-id",