Skip to content

fix(lookup): fix metadata extraction and error handling #107

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

Merged
kvaps merged 1 commit into from
Feb 2, 2026
Merged

fix(lookup): fix metadata extraction and error handling #107

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
38 changes: 24 additions & 14 deletions pkg/engine/engine.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,11 @@ import (
"path/filepath"
"reflect"
"regexp"
"strconv"
"strings"
"unsafe"

"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
"gopkg.in/yaml.v3"

"github.com/cosi-project/runtime/pkg/resource"
Expand Down Expand Up @@ -562,20 +563,21 @@ func readUnexportedField(field reflect.Value) any {

// builds resource with metadata, spec and stringSpec fields
func extractResourceData(r resource.Resource) (map[string]interface{}, error) {
// extract metadata
o, err := resource.MarshalYAML(r)
if err != nil {
return nil, fmt.Errorf("failed to marshal resource to YAML: %w", err)
}
m, err := yaml.Marshal(o)
if err != nil {
return nil, fmt.Errorf("failed to marshal metadata to YAML: %w", err)
}
res := make(map[string]interface{})
if err := yaml.Unmarshal(m, &res); err != nil {
return nil, fmt.Errorf("failed to unmarshal metadata: %w", err)

// Extract metadata directly from resource methods
md := r.Metadata()
metadata := map[string]interface{}{
"namespace": string(md.Namespace()),
"type": string(md.Type()),
"id": string(md.ID()),
"version": md.Version().String(),
"phase": md.Phase().String(),
"owner": string(md.Owner()),
}

res["metadata"] = metadata

// extract spec
val := reflect.ValueOf(r.Spec())
if val.Kind() == reflect.Ptr {
Expand Down Expand Up @@ -606,6 +608,13 @@ func newLookupFunction(ctx context.Context, c *client.Client) func(resource stri

callbackResource := func(parentCtx context.Context, hostname string, r resource.Resource, callError error) error {
if callError != nil {
// Ignore NotFound and PermissionDenied errors - resource doesn't exist or is not accessible
errCode := status.Code(callError)
errStr := callError.Error()
if errCode == codes.NotFound || errCode == codes.PermissionDenied ||
strings.Contains(errStr, "code = NotFound") || strings.Contains(errStr, "code = PermissionDenied") {
return nil
}
Comment on lines +612 to +617
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Jan 30, 2026

Choose a reason for hiding this comment

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

medium

This logic can be made clearer by explicitly handling gRPC status errors separately from the string-matching fallback. Using status.FromError provides a clean way to check if the error is a gRPC status error. This refactoring improves readability by making the two cases (native gRPC error vs. string-based error) distinct. The string matching is brittle and should ideally only be a fallback when a proper status code isn't available.

Suggested change
errCode := status.Code(callError)
errStr := callError.Error()
if errCode == codes.NotFound || errCode == codes.PermissionDenied ||
strings.Contains(errStr, "code = NotFound") || strings.Contains(errStr, "code = PermissionDenied") {
return nil
}
st, ok := status.FromError(callError)
if ok {
code := st.Code()
if code == codes.NotFound || code == codes.PermissionDenied {
return nil
}
} else {
// Fallback for non-gRPC errors that have gRPC error strings.
errStr := callError.Error()
if strings.Contains(errStr, "code = NotFound") || strings.Contains(errStr, "code = PermissionDenied") {
return nil
}
}

Comment on lines +611 to +617
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Jan 30, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🌐 Web query:

grpc-go v1.77.0 recommended way to detect NotFound PermissionDenied wrapped errors errors.As GRPCStatus

💡 Result:

In grpc-go v1.77.0, the simplest/recommended way to detect gRPC codes (including when the error is wrapped) is to use status.Code(err) and compare against codes.NotFound, codes.PermissionDenied, etc. status.Code (and status.FromError/status.Convert) will successfully extract the gRPC Status if the error is a gRPC status error or wraps one. [1]

import (
  "google.golang.org/grpc/codes"
  "google.golang.org/grpc/status"
)

switch status.Code(err) {
case codes.NotFound:
  // ...
case codes.PermissionDenied:
  // ...
case codes.OK:
  // err == nil
default:
  // Unknown / other
}

If you also want the full Status (message/details), use status.Convert(err) or status.FromError(err):

st := status.Convert(err) // convenience wrapper around FromError
if st.Code() == codes.NotFound { /* ... */ }

Notes:

  • status.FromError explicitly documents that it returns the Status when the error “implements GRPCStatus() *Status … or if err wraps a type satisfying this”. [1]
  • In v1.77.0, the module version is 1.77.0 (grpc.Version). [2]

Sources: [1] [2]

🏁 Script executed:

# Inspect the file and context around the mentioned lines
head -n 650 pkg/engine/engine.go | tail -n 80

Repository: cozystack/talm

Length of output: 2661

🏁 Script executed:

# Check imports in the file
head -n 50 pkg/engine/engine.go

Repository: cozystack/talm

Length of output: 1470

🏁 Script executed:

# Search for other error handling patterns in the file
rg "status\.Code|strings\.Contains.*code" pkg/engine/engine.go -A 2 -B 2

Repository: cozystack/talm

Length of output: 450

Remove brittle string matching; status.Code() is already the correct approach for wrapped errors.

The code already uses status.Code(callError), which is the recommended way in grpc-go v1.77.0 to detect gRPC codes on both direct and wrapped errors. The additional string matching with strings.Contains() is unnecessary and should be removed.

♻️ Simplify to 
 			if callError != nil {
 				// Ignore NotFound and PermissionDenied errors - resource doesn't exist or is not accessible
 				errCode := status.Code(callError)
-				errStr := callError.Error()
-				if errCode == codes.NotFound || errCode == codes.PermissionDenied ||
-					strings.Contains(errStr, "code = NotFound") || strings.Contains(errStr, "code = PermissionDenied") {
+				if errCode == codes.NotFound || errCode == codes.PermissionDenied {
 					return nil
 				}
🤖 Prompt for AI Agents 
In `@pkg/engine/engine.go` around lines 611 - 617, The code is doing brittle
string matching against the error text; remove that and rely solely on gRPC's
status.Code for wrapped errors: in the block referencing status.Code(callError)
and strings.Contains, drop the errStr := callError.Error() and both
strings.Contains(...) checks and keep only the comparison of errCode to
codes.NotFound and codes.PermissionDenied (returning nil in that case); also
remove any now-unused import of "strings" if present.

multiErr = multierror.Append(multiErr, callError)
return nil
}
Expand Down Expand Up @@ -636,9 +645,10 @@ func newLookupFunction(ctx context.Context, c *client.Client) func(resource stri
if id != "" && len(resources) == 1 {
return resources[0], nil
}
items := map[string]interface{}{}
// Return items as a slice for proper range iteration in templates
items := make([]interface{}, len(resources))
for i, res := range resources {
items["_"+strconv.Itoa(i)] = res
items[i] = res
}
return map[string]interface{}{
"apiVersion": "v1",
Expand Down