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

pubsub/azuresb: minor comments and fixes #3370

Merged
merged 1 commit into from
Dec 21, 2023
Merged

pubsub/azuresb: minor comments and fixes #3370

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
29 changes: 13 additions & 16 deletions pubsub/azuresb/azuresb.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@
// For pubsub.OpenTopic and pubsub.OpenSubscription, azuresb registers
// for the scheme "azuresb".
// The default URL opener will use a Service Bus Connection String based on
// AZURE_SERVICEBUS_HOSTNAME or SERVICEBUS_CONNECTION_STRING environment variables. SERVICEBUS_CONNECTION_STRING takes precedence.
// AZURE_SERVICEBUS_HOSTNAME or SERVICEBUS_CONNECTION_STRING environment variables.
// SERVICEBUS_CONNECTION_STRING takes precedence.
// To customize the URL opener, or for more details on the URL format,
// see URLOpener.
// See https://gocloud.dev/concepts/urls/ for background information.
Expand Down Expand Up @@ -113,7 +114,7 @@
cs := os.Getenv("SERVICEBUS_CONNECTION_STRING")
sbHostname := os.Getenv("AZURE_SERVICEBUS_HOSTNAME")
if cs == "" && sbHostname == "" {
o.err = errors.New("SERVICEBUS_CONNECTION_STRING or AZURE_SERVICEBUS_HOSTNAME environment variables not set")
o.err = errors.New("Neither SERVICEBUS_CONNECTION_STRING nor AZURE_SERVICEBUS_HOSTNAME environment variables are set")

Check warning on line 117 in pubsub/azuresb/azuresb.go

View check run for this annotation

Codecov / codecov/patch

pubsub/azuresb/azuresb.go#L117 

Added line #L117 was not covered by tests
return
}
o.opener = &URLOpener{ConnectionString: cs, ServiceBusHostname: sbHostname}
Expand Down Expand Up @@ -154,7 +155,7 @@
// https://docs.microsoft.com/en-us/azure/service-bus-messaging/service-bus-dotnet-get-started-with-queues
ConnectionString string

// Azure ServiceBus hostname
// Azure ServiceBus hostname.
// https://learn.microsoft.com/en-us/azure/service-bus-messaging/service-bus-go-how-to-use-queues?tabs=bash
ServiceBusHostname string

Expand All @@ -173,11 +174,10 @@

func (o *URLOpener) sbClient(kind string, u *url.URL) (*servicebus.Client, error) {
if o.ConnectionString == "" && o.ServiceBusHostname == "" {
return nil, fmt.Errorf("open %s %v: ConnectionString or ServiceBusHostname is required", kind, u)
return nil, fmt.Errorf("open %s %v: one of ConnectionString or ServiceBusHostname is required", kind, u)

Check warning on line 177 in pubsub/azuresb/azuresb.go

View check run for this annotation

Codecov / codecov/patch

pubsub/azuresb/azuresb.go#L177 

Added line #L177 was not covered by tests
}

// auth using shared key (old method)
// ConnectionString approach takes presendence
// Auth using shared key.
if o.ConnectionString != "" {
client, err := NewClientFromConnectionString(o.ConnectionString, o.ServiceBusClientOptions)
if err != nil {
Expand All @@ -186,15 +186,12 @@
return client, nil
}

// auth using Azure AAD Workload Identity/AAD Pod Identities/AKS Kubelet Identity/Service Principal
if o.ServiceBusHostname != "" {
client, err := NewClientFromServiceBusHostname(o.ServiceBusHostname, o.ServiceBusClientOptions)
if err != nil {
return nil, fmt.Errorf("open %s %v: invalid service bus hostname %q: %v", kind, u, o.ServiceBusHostname, err)
}
return client, nil
// Auth using Azure AAD Workload Identity/AAD Pod Identities/AKS Kubelet Identity/Service Principal.
client, err := NewClientFromServiceBusHostname(o.ServiceBusHostname, o.ServiceBusClientOptions)
if err != nil {
return nil, fmt.Errorf("open %s %v: invalid service bus hostname %q: %v", kind, u, o.ServiceBusHostname, err)

Check warning on line 192 in pubsub/azuresb/azuresb.go

View check run for this annotation

Codecov / codecov/patch

pubsub/azuresb/azuresb.go#L190-L192 

Added lines #L190 - L192 were not covered by tests
}
return nil, fmt.Errorf("open %s: please set ServiceBusHostname or ConnectionString", kind)
return client, nil

Check warning on line 194 in pubsub/azuresb/azuresb.go

View check run for this annotation

Codecov / codecov/patch

pubsub/azuresb/azuresb.go#L194 

Added line #L194 was not covered by tests
}

// OpenTopicURL opens a pubsub.Topic based on u.
Expand Down Expand Up @@ -256,13 +253,13 @@
BatcherOptions batcher.Options
}

// NewClientFromConnectionString returns a *servicebus.Client from a Service Bus connection string.(using shared key for auth)
// NewClientFromConnectionString returns a *servicebus.Client from a Service Bus connection string, using shared key for auth.
// https://docs.microsoft.com/en-us/azure/service-bus-messaging/service-bus-dotnet-get-started-with-queues
func NewClientFromConnectionString(connectionString string, opts *servicebus.ClientOptions) (*servicebus.Client, error) {
return servicebus.NewClientFromConnectionString(connectionString, opts)
}

// NewClientFromConnectionString returns a *servicebus.Client from a Service Bus connection string.(using shared key for auth)
// NewClientFromConnectionString returns a *servicebus.Client from a Service Bus connection string, using shared key for auth.
// for example you can use workload identity autorization.
// https://learn.microsoft.com/en-us/azure/service-bus-messaging/service-bus-go-how-to-use-queues?tabs=bash
func NewClientFromServiceBusHostname(serviceBusHostname string, opts *servicebus.ClientOptions) (*servicebus.Client, error) {
Expand Down
Loading