Skip to content

Commit cfea96b

Browse files
committed
Bug Bash feedback
1 parent bf7bb16 commit cfea96b

File tree

13 files changed

+91
-31
lines changed

13 files changed

+91
-31
lines changed

cmd/modern/root.go

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ type Root struct {
2525
// It also provides usage examples for sqlcmd.
2626
func (c *Root) DefineCommand(...cmdparser.CommandOptions) {
2727
// Example usage steps
28-
steps := []string{"sqlcmd create mssql --using https://aka.ms/AdventureWorksLT.bak"}
28+
steps := []string{"sqlcmd create mssql --accept-eula --using https://aka.ms/AdventureWorksLT.bak"}
2929

3030
if runtime.GOOS == "windows" {
3131
steps = append(steps, "sqlcmd open ads")
@@ -39,8 +39,11 @@ func (c *Root) DefineCommand(...cmdparser.CommandOptions) {
3939
Steps: steps}}
4040

4141
commandOptions := cmdparser.CommandOptions{
42-
Use: "sqlcmd",
43-
Short: "sqlcmd: Install/Create/Query SQL Server, Azure SQL, and Tools",
42+
Use: "sqlcmd",
43+
Short: `sqlcmd: Install/Create/Query SQL Server, Azure SQL, and Tools
44+
45+
Feedback:
46+
https://github.com/microsoft/go-sqlcmd/issues/new`,
4447
SubCommands: c.SubCommands(),
4548
Examples: examples,
4649
}
@@ -86,6 +89,14 @@ func (c *Root) IsValidSubCommand(command string) bool {
8689
}
8790

8891
func (c *Root) addGlobalFlags() {
92+
var unused bool
93+
c.AddFlag(cmdparser.FlagOptions{
94+
Bool: &unused,
95+
Name: "?",
96+
Shorthand: "?",
97+
Usage: "help for backwards compatibility flags (-S, -U, -E etc.)",
98+
})
99+
89100
c.AddFlag(cmdparser.FlagOptions{
90101
String: &c.configFilename,
91102
DefaultString: config.DefaultFileName(),

cmd/modern/root/config.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,10 @@
44
package root
55

66
import (
7+
"fmt"
78
"github.com/microsoft/go-sqlcmd/cmd/modern/root/config"
89
"github.com/microsoft/go-sqlcmd/internal/cmdparser"
10+
"github.com/microsoft/go-sqlcmd/internal/pal"
911
)
1012

1113
// Config defines the `sqlcmd config` sub-commands
@@ -20,6 +22,17 @@ func (c *Config) DefineCommand(...cmdparser.CommandOptions) {
2022
Use: "config",
2123
Short: `Modify sqlconfig files using subcommands like "sqlcmd config use-context mssql"`,
2224
SubCommands: c.SubCommands(),
25+
Examples: []cmdparser.ExampleOptions{
26+
{
27+
Description: "Add context for existing endpoint and user",
28+
Steps: []string{
29+
fmt.Sprintf("%s SQLCMD_PASSWORD=<password>", pal.CreateEnvVarKeyword()),
30+
"sqlcmd config add-user --name sa1434 --username sa",
31+
fmt.Sprintf("%s SQLCMD_PASSWORD=", pal.CreateEnvVarKeyword()),
32+
"sqlcmd config add-endpoint --name ep1434 --address localhost --port 1434",
33+
"sqlcmd config add-context --name mssql1434 --user sa1434 --endpoint ep1434"},
34+
},
35+
},
2336
}
2437

2538
c.Cmd.DefineCommand(options)

cmd/modern/root/config/add-context.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,10 @@ func (c *AddContext) DefineCommand(...cmdparser.CommandOptions) {
2626
Short: "Add a context",
2727
Examples: []cmdparser.ExampleOptions{
2828
{
29-
Description: "Add a default context",
29+
Description: "Add a context for a local instance of SQL Server on port 1433 using trusted authentication",
3030
Steps: []string{
3131
"sqlcmd config add-endpoint --name localhost-1433",
32-
"sqlcmd config add-context --name my-context --endpoint localhost-1433"}},
32+
"sqlcmd config add-context --name mssql --endpoint localhost-1433"}},
3333
},
3434
Run: c.run}
3535

@@ -44,12 +44,12 @@ func (c *AddContext) DefineCommand(...cmdparser.CommandOptions) {
4444
c.AddFlag(cmdparser.FlagOptions{
4545
String: &c.endpointName,
4646
Name: "endpoint",
47-
Usage: "Name of endpoint this context will use, use `sqlcmd config get-endpoints` to see list"})
47+
Usage: "Name of endpoint this context will use"})
4848

4949
c.AddFlag(cmdparser.FlagOptions{
5050
String: &c.userName,
5151
Name: "user",
52-
Usage: "Name of user this context will use, use `sqlcmd config get-users` to see list"})
52+
Usage: "Name of user this context will use"})
5353
}
5454

5555
// run adds a context to the configuration and sets it as the current context. The

cmd/modern/root/config/add-user.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@
44
package config
55

66
import (
7+
"fmt"
78
"github.com/microsoft/go-sqlcmd/cmd/modern/sqlconfig"
9+
"github.com/microsoft/go-sqlcmd/internal/pal"
810
"os"
911

1012
"github.com/microsoft/go-sqlcmd/internal/cmdparser"
@@ -30,8 +32,10 @@ func (c *AddUser) DefineCommand(...cmdparser.CommandOptions) {
3032
{
3133
Description: "Add a user",
3234
Steps: []string{
33-
`SET SQLCMD_PASSWORD="AComp!exPa$$w0rd"`,
34-
"sqlcmd config add-user --name my-user --name user1"},
35+
fmt.Sprintf(`%s SQLCMD_PASSWORD=<password>`, pal.CreateEnvVarKeyword()),
36+
"sqlcmd config add-user --name my-user --username user1",
37+
fmt.Sprintf(`%s SQLCMD_PASSWORD=`, pal.CreateEnvVarKeyword()),
38+
},
3539
},
3640
},
3741
Run: c.run}

cmd/modern/root/config/add-user_windows.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,6 @@ func (c *AddUser) encryptPasswordFlag() {
99
c.AddFlag(cmdparser.FlagOptions{
1010
Bool: &c.encryptPassword,
1111
Name: "encrypt-password",
12-
Usage: "Encode the password",
12+
Usage: "Encrypt the password",
1313
})
1414
}

cmd/modern/root/config/view.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,15 +21,15 @@ func (c *View) DefineCommand(...cmdparser.CommandOptions) {
2121
Short: "Display merged sqlconfig settings or a specified sqlconfig file",
2222
Examples: []cmdparser.ExampleOptions{
2323
{
24-
Description: "Show merged sqlconfig settings",
24+
Description: "Show sqlconfig settings, with REDACTED authentication data",
2525
Steps: []string{"sqlcmd config view"},
2626
},
2727
{
28-
Description: "Show merged sqlconfig settings and raw authentication data",
28+
Description: "Show sqlconfig settings and raw authentication data",
2929
Steps: []string{"sqlcmd config view --raw"},
3030
},
3131
},
32-
Aliases: []string{"use", "change-context", "set-context"},
32+
Aliases: []string{"show"},
3333
Run: c.run,
3434
}
3535

cmd/modern/root/open/ads.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,13 +38,13 @@ func (c *Ads) DefineCommand(...cmdparser.CommandOptions) {
3838
func (c *Ads) run() {
3939
endpoint, user := config.CurrentContext()
4040

41+
hostname := endpoint.EndpointDetails.Address
42+
4143
// If the context has a local container, ensure it is running, otherwise bail out
4244
if endpoint.AssetDetails != nil && endpoint.AssetDetails.ContainerDetails != nil {
4345
c.ensureContainerIsRunning(endpoint)
4446
}
4547

46-
hostname := endpoint.EndpointDetails.Address
47-
4848
// If the hostname is localhost, ADS will not connect to the container
4949
// because it will try to connect to the host machine. To work around
5050
// BUG(stuartpa): I think this might be a bug in ADS?
@@ -75,6 +75,7 @@ func (c *Ads) ensureContainerIsRunning(endpoint sqlconfig.Endpoint) {
7575
// launchAds launches the Azure Data Studio using the specified server and username.
7676
func (c *Ads) launchAds(host string, port int, username string) {
7777
output := c.Output()
78+
7879
args := []string{
7980
"-r",
8081
fmt.Sprintf(

cmd/modern/root/open/ads_windows.go

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,10 @@ package open
22

33
import (
44
"fmt"
5-
"github.com/99designs/keyring"
65
"github.com/microsoft/go-sqlcmd/cmd/modern/sqlconfig"
76
"github.com/microsoft/go-sqlcmd/internal/cmdparser"
87
"github.com/microsoft/go-sqlcmd/internal/credman"
8+
"github.com/microsoft/go-sqlcmd/internal/secret"
99
)
1010

1111
// Type Ads is used to implement the "open ads" which launches Azure
@@ -33,7 +33,7 @@ func (c *Ads) persistCredentialForAds(
3333

3434
// Store the SQL password in the Windows Credential Manager with the
3535
// generated target name
36-
/*c.credential = credman.Credential{
36+
c.credential = credman.Credential{
3737
TargetName: targetName,
3838
CredentialBlob: secret.DecodeAsUtf16(
3939
user.BasicAuth.Password, user.BasicAuth.PasswordEncrypted),
@@ -43,15 +43,6 @@ func (c *Ads) persistCredentialForAds(
4343

4444
c.removePreviousCredential()
4545
c.writeCredential()
46-
*/
47-
ring, _ := keyring.Open(keyring.Config{
48-
ServiceName: "example",
49-
})
50-
51-
_ = ring.Set(keyring.Item{
52-
Key: "foo",
53-
Data: []byte("secret-bar"),
54-
})
5546
}
5647

5748
// adsKey returns the credential target name for the given instance, database,
@@ -86,6 +77,13 @@ func (c *Ads) removePreviousCredential() {
8677

8778
// writeCredential stores the current instance's credential in the Windows Credential Manager
8879
func (c *Ads) writeCredential() {
80+
output := c.Output()
81+
8982
err := credman.WriteCredential(&c.credential, credman.CredTypeGeneric)
90-
c.CheckErr(err)
83+
if err != nil {
84+
output.FatalfErrorWithHints(
85+
err,
86+
[]string{"A 'Not enough memory resources are available' error can be caused by too many credentials already stored in Windows Credential Manager"},
87+
"Failed to write credential to Windows Credential Manager")
88+
}
9189
}

cmd/sqlcmd/sqlcmd.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ type SQLCmdArguments struct {
6161
MultiSubnetFailover bool `short:"M" help:"Provided for backward compatibility. Sqlcmd always optimizes detection of the active replica of a SQL Failover Cluster."`
6262
Password string `short:"P" help:"Obsolete. The initial passwords must be set using the SQLCMDPASSWORD environment variable or entered at the password prompt."`
6363
// Keep Help at the end of the list
64-
Help bool `short:"?" help:"Show syntax summary."`
64+
Help bool `short:"?" help:"-? shows this syntax summary, --help shows modern sqlcmd sub-command help"`
6565
}
6666

6767
// Validate accounts for settings not described by Kong attributes

go.mod

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,28 +16,34 @@ require (
1616
github.com/spf13/cobra v1.6.1
1717
github.com/spf13/viper v1.14.0
1818
github.com/stretchr/testify v1.8.1
19-
golang.org/x/sys v0.0.0-20220908164124-27713097b956
19+
golang.org/x/sys v0.3.0
2020
golang.org/x/text v0.4.0
2121
gopkg.in/yaml.v2 v2.4.0
2222
)
2323

2424
require (
25+
github.com/99designs/go-keychain v0.0.0-20191008050251-8e49817e8af4 // indirect
26+
github.com/99designs/keyring v1.2.2 // indirect
2527
github.com/Azure/azure-sdk-for-go/sdk/azcore v1.1.2 // indirect
2628
github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.1.0 // indirect
2729
github.com/Azure/azure-sdk-for-go/sdk/internal v1.0.0 // indirect
2830
github.com/AzureAD/microsoft-authentication-library-for-go v0.5.1 // indirect
2931
github.com/Microsoft/go-winio v0.6.0 // indirect
3032
github.com/beorn7/perks v1.0.1 // indirect
33+
github.com/danieljoos/wincred v1.1.2 // indirect
3134
github.com/davecgh/go-spew v1.1.1 // indirect
3235
github.com/docker/go-metrics v0.0.1 // indirect
3336
github.com/docker/go-units v0.5.0 // indirect
3437
github.com/docker/libtrust v0.0.0-20160708172513-aabc10ec26b7 // indirect
38+
github.com/dvsekhvalnov/jose2go v1.5.0 // indirect
3539
github.com/fsnotify/fsnotify v1.6.0 // indirect
40+
github.com/godbus/dbus v0.0.0-20190726142602-4481cbc300e2 // indirect
3641
github.com/gogo/protobuf v1.3.2 // indirect
3742
github.com/golang-jwt/jwt v3.2.2+incompatible // indirect
3843
github.com/golang-sql/civil v0.0.0-20190719163853-cb61b32ac6fe // indirect
3944
github.com/golang/protobuf v1.5.2 // indirect
4045
github.com/gorilla/mux v1.8.0 // indirect
46+
github.com/gsterjov/go-libsecret v0.0.0-20161001094733-a6f4afe4910c // indirect
4147
github.com/hashicorp/hcl v1.0.0 // indirect
4248
github.com/inconshreveable/mousetrap v1.0.1 // indirect
4349
github.com/kylelemons/godebug v1.1.0 // indirect
@@ -47,6 +53,7 @@ require (
4753
github.com/mitchellh/mapstructure v1.5.0 // indirect
4854
github.com/moby/term v0.0.0-20221120202655-abb19827d345 // indirect
4955
github.com/morikuni/aec v1.0.0 // indirect
56+
github.com/mtibben/percent v0.2.1 // indirect
5057
github.com/opencontainers/go-digest v1.0.0 // indirect
5158
github.com/opencontainers/image-spec v1.0.2 // indirect
5259
github.com/pelletier/go-toml v1.9.5 // indirect
@@ -66,6 +73,7 @@ require (
6673
golang.org/x/crypto v0.0.0-20220622213112-05595931fe9d // indirect
6774
golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4 // indirect
6875
golang.org/x/net v0.0.0-20221014081412-f15817d10f9b // indirect
76+
golang.org/x/term v0.3.0 // indirect
6977
golang.org/x/tools v0.1.12 // indirect
7078
google.golang.org/protobuf v1.28.1 // indirect
7179
gopkg.in/ini.v1 v1.67.0 // indirect

0 commit comments

Comments
 (0)