Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 9 additions & 4 deletions cmd/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ const notSetValue = "[not set]"
// It allows setting or clearing Glean credentials and connection settings.
type ConfigOptions struct {
host string // Glean instance hostname
port string // Glean instance port
token string // API token for authentication
email string // User's email address
clear bool // Whether to clear all configuration
Expand All @@ -38,6 +39,9 @@ func NewCmdConfig() *cobra.Command {
glean config --host linkedin
glean config --host linkedin-be.glean.com

# Set Glean host and port (e.g. custom proxy)
glean config --host foo.bar.com --port 7960

# Set Glean API token
glean config --token your-token

Expand All @@ -59,8 +63,8 @@ func NewCmdConfig() *cobra.Command {

fmt.Println("Current configuration:")
fmt.Printf(" %-10s %s\n", "Host:", valueOrNotSet(cfg.GleanHost))
fmt.Printf(" %-10s %s\n", "Port:", valueOrNotSet(cfg.GleanPort))
fmt.Printf(" %-10s %s\n", "Email:", valueOrNotSet(cfg.GleanEmail))

// Mask token if present
tokenDisplay := notSetValue
if cfg.GleanToken != "" {
Expand All @@ -78,11 +82,11 @@ func NewCmdConfig() *cobra.Command {
return nil
}

if opts.host == "" && opts.token == "" && opts.email == "" {
return fmt.Errorf("no configuration provided. Use --host, --token, or --email to set configuration")
if opts.host == "" && opts.port == "" && opts.token == "" && opts.email == "" {
return fmt.Errorf("no configuration provided. Use --host, --port, --token, or --email to set configuration")
}

if err := config.SaveConfig(opts.host, opts.token, opts.email); err != nil {
if err := config.SaveConfig(opts.host, opts.port, opts.token, opts.email); err != nil {
return fmt.Errorf("failed to save configuration: %w", err)
}

Expand All @@ -92,6 +96,7 @@ func NewCmdConfig() *cobra.Command {
}

cmd.Flags().StringVar(&opts.host, "host", "", "Glean instance name or full hostname (e.g., 'linkedin' or 'linkedin-be.glean.com')")
cmd.Flags().StringVar(&opts.port, "port", "", "Glean instance port (e.g., '8080' for custom proxy or local development)")
cmd.Flags().StringVar(&opts.token, "token", "", "Glean API token")
cmd.Flags().StringVar(&opts.email, "email", "", "Email address for API requests")
cmd.Flags().BoolVar(&opts.clear, "clear", false, "Clear all stored credentials")
Expand Down
27 changes: 18 additions & 9 deletions internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,15 @@ var ConfigPath string

const (
hostKey = "host"
portKey = "port"
tokenKey = "token"
emailKey = "email"
)

// Config holds the Glean API credentials and connection settings.
type Config struct {
GleanHost string `json:"host"`
GleanPort string `json:"port"`
GleanToken string `json:"token"`
GleanEmail string `json:"email"`
}
Expand All @@ -54,14 +56,6 @@ func ValidateAndTransformHost(host string) (string, error) {
return fmt.Sprintf("%s-be.glean.com", host), nil
}

if !strings.HasSuffix(host, ".glean.com") {
return "", fmt.Errorf("invalid host format. Must be either 'instance' or 'instance-be.glean.com'")
}

if !strings.HasSuffix(strings.TrimSuffix(host, ".glean.com"), "-be") {
return "", fmt.Errorf("invalid host format. Must end with '-be.glean.com'")
}

Comment on lines -57 to -64
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing this allows usage of custom domains or IP's which is important to support the proxy use case.

return host, nil
}

Expand All @@ -83,7 +77,7 @@ func LoadConfig() (*Config, error) {

// SaveConfig stores configuration in both the system keyring and file storage.
// It returns an error only if both storage methods fail.
func SaveConfig(host, token, email string) error {
func SaveConfig(host, port, token, email string) error {
if host != "" {
validHost, err := ValidateAndTransformHost(host)
if err != nil {
Expand All @@ -98,6 +92,11 @@ func SaveConfig(host, token, email string) error {
keyringErr = err
}
}
if port != "" {
if err := keyringImpl.Set(ServiceName, portKey, port); err != nil {
keyringErr = err
}
}
if token != "" {
if err := keyringImpl.Set(ServiceName, tokenKey, token); err != nil {
keyringErr = err
Expand All @@ -118,6 +117,9 @@ func SaveConfig(host, token, email string) error {
if host != "" {
cfg.GleanHost = host
}
if port != "" {
cfg.GleanPort = port
}
if token != "" {
cfg.GleanToken = token
}
Expand All @@ -139,6 +141,9 @@ func ClearConfig() error {
if err := keyringImpl.Delete(ServiceName, hostKey); err != nil && err != keyring.ErrNotFound {
keyringErr = err
}
if err := keyringImpl.Delete(ServiceName, portKey); err != nil && err != keyring.ErrNotFound {
keyringErr = err
}
if err := keyringImpl.Delete(ServiceName, tokenKey); err != nil && err != keyring.ErrNotFound {
keyringErr = err
}
Expand Down Expand Up @@ -193,6 +198,10 @@ func loadFromKeyring() *Config {
cfg.GleanHost = host
}

if port, err := keyringImpl.Get(ServiceName, portKey); err == nil {
cfg.GleanPort = port
}

if token, err := keyringImpl.Get(ServiceName, tokenKey); err == nil {
cfg.GleanToken = token
}
Expand Down
26 changes: 4 additions & 22 deletions internal/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,18 +130,6 @@ func TestValidateAndTransformHost(t *testing.T) {
input: "linkedin-be.glean.com",
want: "linkedin-be.glean.com",
},
{
name: "invalid domain",
input: "linkedin.example.com",
wantErr: true,
errContains: "invalid host format",
},
{
name: "missing -be suffix",
input: "linkedin.glean.com",
wantErr: true,
errContains: "invalid host format",
},
Comment on lines -133 to -144
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests are removed because we actually do support this case now

}

for _, tt := range tests {
Expand Down Expand Up @@ -178,7 +166,7 @@ func TestConfigOperations(t *testing.T) {

t.Run("save and load config with working keyring", func(t *testing.T) {
// Save config
err := SaveConfig("linkedin", "test-token", "test@example.com")
err := SaveConfig("linkedin", "", "test-token", "test@example.com")
require.NoError(t, err)

// Load config
Expand All @@ -195,7 +183,7 @@ func TestConfigOperations(t *testing.T) {

t.Run("fallback to config file when keyring fails", func(t *testing.T) {
// First save config successfully
err := SaveConfig("linkedin", "test-token", "test@example.com")
err := SaveConfig("linkedin", "", "test-token", "test@example.com")
require.NoError(t, err)

// Now simulate keyring failure
Expand All @@ -212,7 +200,7 @@ func TestConfigOperations(t *testing.T) {

t.Run("clear config removes from both storages", func(t *testing.T) {
// First save some config
err := SaveConfig("linkedin", "test-token", "test@example.com")
err := SaveConfig("linkedin", "", "test-token", "test@example.com")
require.NoError(t, err)

// Reset mock error
Expand All @@ -238,12 +226,6 @@ func TestConfigOperations(t *testing.T) {
assert.Empty(t, cfg.GleanEmail)
})

t.Run("save invalid host", func(t *testing.T) {
err := SaveConfig("invalid.example.com", "test-token", "test@example.com")
assert.Error(t, err)
assert.Contains(t, err.Error(), "invalid host format")
})

t.Run("save with both storages failing", func(t *testing.T) {
// Simulate keyring failure
mock.err = assert.AnError
Expand All @@ -267,7 +249,7 @@ func TestConfigOperations(t *testing.T) {
os.MkdirAll(configDir, 0700)
}()

err = SaveConfig("linkedin", "test-token", "test@example.com")
err = SaveConfig("linkedin", "", "test-token", "test@example.com")
assert.Error(t, err)
assert.Contains(t, err.Error(), "failed to save config")
assert.Contains(t, err.Error(), "keyring error")
Expand Down
13 changes: 12 additions & 1 deletion internal/http/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,17 @@ func NewClient(cfg *config.Config) (Client, error) {
return NewClientFunc(cfg)
}

func buildBaseURL(cfg *config.Config) string {
host := cfg.GleanHost
port := cfg.GleanPort

if port != "" {
return fmt.Sprintf("https://%s:%s", host, port)
}

return fmt.Sprintf("https://%s", host)
}

// defaultNewClient is the default implementation of NewClient
func defaultNewClient(cfg *config.Config) (Client, error) {
if cfg.GleanHost == "" {
Expand All @@ -66,7 +77,7 @@ func defaultNewClient(cfg *config.Config) (Client, error) {
return nil, fmt.Errorf("Glean token not configured. Run 'glean config --token <token>' to set it")
}

baseURL := fmt.Sprintf("https://%s", cfg.GleanHost)
baseURL := buildBaseURL(cfg)

return &client{
http: &http.Client{},
Expand Down
89 changes: 89 additions & 0 deletions internal/http/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,45 @@ func (m *mockHTTPClient) Do(req *http.Request) (*http.Response, error) {
return nil, fmt.Errorf("doFunc not implemented")
}

func TestBuildBaseURL(t *testing.T) {
tests := []struct {
name string
config *config.Config
expected string
}{
{
name: "host only",
config: &config.Config{
GleanHost: "test-be.glean.com",
},
expected: "https://test-be.glean.com",
},
{
name: "host with port",
config: &config.Config{
GleanHost: "foo.bar.com",
GleanPort: "8080",
},
expected: "https://foo.bar.com:8080",
},
{
name: "host with empty port",
config: &config.Config{
GleanHost: "test-be.glean.com",
GleanPort: "",
},
expected: "https://test-be.glean.com",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := buildBaseURL(tt.config)
assert.Equal(t, tt.expected, result)
})
}
}

func TestNewClient(t *testing.T) {
// Save original NewClientFunc and restore after test
originalNewClientFunc := NewClientFunc
Expand Down Expand Up @@ -98,6 +137,56 @@ func TestSendRequest(t *testing.T) {
GleanEmail: "test@example.com",
}

t.Run("successful GET request with foo.bar.com:7960 ", func(t *testing.T) {
mock := &mockHTTPClient{
doFunc: func(req *http.Request) (*http.Response, error) {
// Verify request
assert.Equal(t, "GET", req.Method)
assert.Equal(t, "Bearer test-token", req.Header.Get("Authorization"))
assert.Equal(t, "test@example.com", req.Header.Get("X-Scio-Actas"))
assert.Equal(t, "string", req.Header.Get("X-Glean-Auth-Type"))
assert.Equal(t, "application/json", req.Header.Get("Content-Type"))

// Verify URL construction
assert.Equal(t, "https://foo.bar.com:7960/rest/api/v1/test", req.URL.String())

// Return mock response
responseBody := map[string]string{"status": "ok"}
jsonBody, _ := json.Marshal(responseBody)
return &http.Response{
StatusCode: http.StatusOK,
Body: io.NopCloser(bytes.NewReader(jsonBody)),
}, nil
},
}

cfgWithPort := &config.Config{
GleanHost: "foo.bar.com",
GleanPort: "7960",
GleanToken: "test-token",
GleanEmail: "test@example.com",
}

client := &client{
http: mock,
baseURL: buildBaseURL(cfgWithPort),
cfg: cfgWithPort,
}

req := &Request{
Method: "GET",
Path: "test",
}

resp, err := client.SendRequest(req)
require.NoError(t, err)

var result map[string]string
err = json.Unmarshal(resp, &result)
require.NoError(t, err)
assert.Equal(t, "ok", result["status"])
})

t.Run("successful GET request with custom headers", func(t *testing.T) {
mock := &mockHTTPClient{
doFunc: func(req *http.Request) (*http.Response, error) {
Expand Down
Loading