Skip to content

Commit

Permalink
fix: fix minor bugs
Browse files Browse the repository at this point in the history
* azure: return error details for failed bucket notification deployments
* azure: provide correct bucket name to notification creator
* azure: don't fail deployments for APIs with no routes
* azure: kv set correct error response code for failed get requests
* return consistent kv not found errors in all providers
* azure: stop returning an error for not found during kv delete
  • Loading branch information
jyecusch authored Feb 26, 2024
1 parent 4d3c77d commit 9fb78ff
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 14 deletions.
2 changes: 1 addition & 1 deletion cloud/aws/runtime/keyvalue/dynamodb.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ func (s *DynamoKeyValueService) Get(ctx context.Context, req *keyvaluepb.KeyValu
if result.Item == nil {
return nil, newErr(
codes.NotFound,
fmt.Sprintf("%v not found", req.Ref),
fmt.Sprintf("key %s not found in store %s", req.Ref.Key, req.Ref.Store),
err,
)
}
Expand Down
6 changes: 6 additions & 0 deletions cloud/azure/deploy/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"strings"

"github.com/nitrictech/nitric/cloud/common/deploy/resources"
"github.com/nitrictech/nitric/core/pkg/logger"
deploymentspb "github.com/nitrictech/nitric/core/pkg/proto/deployments/v1"

"github.com/getkin/kin-openapi/openapi3"
Expand Down Expand Up @@ -81,6 +82,11 @@ func (p *NitricAzurePulumiProvider) Api(ctx *pulumi.Context, parent pulumi.Resou
return fmt.Errorf("invalid document supplied for api: %s", name)
}

if len(openapiDoc.Paths) < 1 {
logger.Warnf("skipping deployment of API %s, no routes defined", name)
return nil
}

managedIdentities := p.containerEnv.ManagedUser.ID().ToStringOutput().ApplyT(func(id string) apimanagement.UserIdentityPropertiesMapOutput {
return apimanagement.UserIdentityPropertiesMap{
id: nil,
Expand Down
8 changes: 4 additions & 4 deletions cloud/azure/deploy/bucket.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,19 +38,19 @@ func eventTypeToStorageEventType(eventType *storagepb.BlobEventType) []string {
func (p *NitricAzurePulumiProvider) newAzureBucketNotification(ctx *pulumi.Context, parent pulumi.Resource, bucketName string, config *deploymentspb.BucketListener) error {
target, ok := p.containerApps[config.GetService()]
if !ok {
return fmt.Errorf("")
return fmt.Errorf("target container app %s not found", config.GetService())
}

bucket, ok := p.buckets[bucketName]
if !ok {
return fmt.Errorf("")
return fmt.Errorf("target bucket %s not found", bucketName)
}

opts := []pulumi.ResourceOption{pulumi.Parent(parent), pulumi.DependsOn([]pulumi.Resource{target.App, bucket})}

hostUrl, err := target.HostUrl()
if err != nil {
return err
return fmt.Errorf("unable to determine container app host URL: %w", err)
}

_, err = pulumiEventgrid.NewEventSubscription(ctx, ResourceName(ctx, bucketName+target.Name, EventSubscriptionRT), &pulumiEventgrid.EventSubscriptionArgs{
Expand Down Expand Up @@ -91,7 +91,7 @@ func (p *NitricAzurePulumiProvider) Bucket(ctx *pulumi.Context, parent pulumi.Re
}

for _, sub := range config.Listeners {
err = p.newAzureBucketNotification(ctx, parent, name+sub.GetService(), sub)
err = p.newAzureBucketNotification(ctx, parent, name, sub)
if err != nil {
return err
}
Expand Down
26 changes: 25 additions & 1 deletion cloud/azure/runtime/keyvalue/keyvalue.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,10 @@ import (
"context"
"encoding/json"
"fmt"
"net/http"
"time"

"github.com/Azure/azure-sdk-for-go/sdk/azcore"
"github.com/Azure/azure-sdk-for-go/sdk/azidentity"
"github.com/Azure/azure-sdk-for-go/sdk/data/aztables"
"github.com/nitrictech/nitric/cloud/azure/runtime/env"
Expand Down Expand Up @@ -68,8 +70,21 @@ func (s *AzureStorageTableKeyValueService) Get(ctx context.Context, req *keyvalu

response, err := client.GetEntity(ctx, req.Ref.Store, req.Ref.Key, nil)
if err != nil {
var respErr *azcore.ResponseError
if errors.As(err, &respErr) {
switch respErr.StatusCode {
case http.StatusNotFound:
// Handle not found error
return nil, newErr(
codes.NotFound,
fmt.Sprintf("key %s not found in store %s", req.Ref.Key, req.Ref.Store),
err,
)
}
}

return nil, newErr(
codes.InvalidArgument,
codes.Unknown,
"failed to call aztables:GetEntity",
err,
)
Expand Down Expand Up @@ -186,6 +201,15 @@ func (s *AzureStorageTableKeyValueService) Delete(ctx context.Context, req *keyv

_, err = client.DeleteEntity(ctx, req.Ref.Store, req.Ref.Key, nil)
if err != nil {
var respErr *azcore.ResponseError
if errors.As(err, &respErr) {
switch respErr.StatusCode {
case http.StatusNotFound:
// not found isn't an error for delete
return &keyvaluepb.KeyValueDeleteResponse{}, nil
}
}

return nil, newErr(
codes.Internal,
"failed to call aztables.DeleteEntity",
Expand Down
22 changes: 14 additions & 8 deletions cloud/gcp/runtime/keyvalue/firestore.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

package document
package keyvalue

import (
"context"
Expand All @@ -23,7 +23,6 @@ import (
v1 "github.com/nitrictech/nitric/core/pkg/proto/keyvalue/v1"

"google.golang.org/grpc/codes"
grpcCodes "google.golang.org/grpc/codes"
"google.golang.org/protobuf/types/known/structpb"

"cloud.google.com/go/firestore"
Expand Down Expand Up @@ -53,10 +52,17 @@ func (s *FirestoreDocService) Get(ctx context.Context, req *v1.KeyValueGetReques

value, err := doc.Get(ctx)
if err != nil {
if status.Code(err) == grpcCodes.PermissionDenied {
switch status.Code(err) {
case codes.NotFound:
return nil, newErr(
codes.NotFound,
fmt.Sprintf("key %s not found in store %s", req.Ref.Key, req.Ref.Store),
err,
)
case codes.PermissionDenied:
return nil, newErr(
codes.PermissionDenied,
"permission denied, have you requested access to this collection?",
"permission denied, have you requested access to this key value store?",
err,
)
}
Expand Down Expand Up @@ -107,10 +113,10 @@ func (s *FirestoreDocService) Set(ctx context.Context, req *v1.KeyValueSetReques
doc := s.getDocRef(req.Ref)

if _, err := doc.Set(ctx, req.Content.AsMap()); err != nil {
if status.Code(err) == grpcCodes.PermissionDenied {
if status.Code(err) == codes.PermissionDenied {
return nil, newErr(
codes.PermissionDenied,
"permission denied, have you requested access to this collection?",
"permission denied, have you requested access to this key value store?",
err,
)
}
Expand Down Expand Up @@ -140,10 +146,10 @@ func (s *FirestoreDocService) Delete(ctx context.Context, req *v1.KeyValueDelete

// Delete document
if _, err := doc.Delete(ctx); err != nil {
if status.Code(err) == grpcCodes.PermissionDenied {
if status.Code(err) == codes.PermissionDenied {
return nil, newErr(
codes.PermissionDenied,
"permission denied, have you requested access to this collection?",
"permission denied, have you requested access to this key value store?",
err,
)
}
Expand Down

0 comments on commit 9fb78ff

Please sign in to comment.