Skip to content

Commit 856c6a2

Browse files
committed
feat: robust logging
On-behalf-of: @SAP [email protected] Signed-off-by: Artem Shcherbatiuk <[email protected]>
1 parent d65d7f2 commit 856c6a2

File tree

13 files changed

+207
-209
lines changed

13 files changed

+207
-209
lines changed

cmd/listener.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ var listenCmd = &cobra.Command{
105105
// Create the appropriate reconciler based on configuration
106106
var reconcilerInstance reconciler.CustomReconciler
107107
if appCfg.EnableKcp {
108-
kcpReconciler, err := kcp.NewKCPReconciler(appCfg, reconcilerOpts, log)
108+
kcpReconciler, err := kcp.NewKCPReconciler(appCfg, reconcilerOpts, *log)
109109
if err != nil {
110110
log.Error().Err(err).Msg("unable to create KCP reconciler")
111111
os.Exit(1)
@@ -123,7 +123,7 @@ var listenCmd = &cobra.Command{
123123

124124
reconcilerInstance = kcpReconciler
125125
} else {
126-
reconcilerInstance, err = clusteraccess.CreateMultiClusterReconciler(appCfg, reconcilerOpts, log)
126+
reconcilerInstance, err = clusteraccess.NewClusterAccessReconciler(ctx, appCfg, reconcilerOpts, nil, nil, *log)
127127
if err != nil {
128128
log.Error().Err(err).Msg("unable to create cluster access reconciler")
129129
os.Exit(1)

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ require (
3434
github.com/stretchr/testify v1.10.0
3535
go.opentelemetry.io/otel v1.37.0
3636
go.opentelemetry.io/otel/trace v1.37.0
37+
golang.org/x/exp v0.0.0-20250620022241-b7579e27df2b
3738
gopkg.in/yaml.v3 v3.0.1
3839
k8s.io/api v0.33.2
3940
k8s.io/apiextensions-apiserver v0.32.4
@@ -116,7 +117,6 @@ require (
116117
go.yaml.in/yaml/v2 v2.4.2 // indirect
117118
go.yaml.in/yaml/v3 v3.0.3 // indirect
118119
golang.org/x/crypto v0.39.0 // indirect
119-
golang.org/x/exp v0.0.0-20250620022241-b7579e27df2b // indirect
120120
golang.org/x/net v0.41.0 // indirect
121121
golang.org/x/oauth2 v0.30.0 // indirect
122122
golang.org/x/sync v0.15.0 // indirect

listener/pkg/apischema/builder.go

Lines changed: 19 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -4,22 +4,21 @@ import (
44
"encoding/json"
55
"errors"
66
"fmt"
7-
"maps"
87
"slices"
98
"strings"
109

11-
"k8s.io/apimachinery/pkg/api/meta"
12-
"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
13-
1410
"github.com/hashicorp/go-multierror"
15-
"github.com/openmfp/golang-commons/logger"
16-
"github.com/openmfp/kubernetes-graphql-gateway/common"
11+
"golang.org/x/exp/maps"
1712
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
13+
"k8s.io/apimachinery/pkg/api/meta"
1814
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
19-
k8sschema "k8s.io/apimachinery/pkg/runtime/schema"
2015
runtimeSchema "k8s.io/apimachinery/pkg/runtime/schema"
2116
"k8s.io/client-go/openapi"
2217
"k8s.io/kube-openapi/pkg/validation/spec"
18+
"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
19+
20+
"github.com/openmfp/golang-commons/logger"
21+
"github.com/openmfp/kubernetes-graphql-gateway/common"
2322
)
2423

2524
var (
@@ -33,18 +32,19 @@ var (
3332
ErrUnmarshalGVK = errors.New("failed to unmarshal GVK extension")
3433
)
3534

36-
// SchemaBuilder helps construct GraphQL field config arguments
3735
type SchemaBuilder struct {
3836
schemas map[string]*spec.Schema
3937
err *multierror.Error
40-
log *logger.Logger
38+
log logger.Logger
4139
}
4240

4341
func NewSchemaBuilder(oc openapi.Client, preferredApiGroups []string) *SchemaBuilder {
44-
return NewSchemaBuilderWithLogger(oc, preferredApiGroups, nil)
42+
// Create a default logger if none provided
43+
defaultLogPtr, _ := logger.New(logger.DefaultConfig())
44+
return NewSchemaBuilderWithLogger(oc, preferredApiGroups, *defaultLogPtr)
4545
}
4646

47-
func NewSchemaBuilderWithLogger(oc openapi.Client, preferredApiGroups []string, log *logger.Logger) *SchemaBuilder {
47+
func NewSchemaBuilderWithLogger(oc openapi.Client, preferredApiGroups []string, log logger.Logger) *SchemaBuilder {
4848
b := &SchemaBuilder{
4949
schemas: make(map[string]*spec.Schema),
5050
log: log,
@@ -59,9 +59,7 @@ func NewSchemaBuilderWithLogger(oc openapi.Client, preferredApiGroups []string,
5959
for path, gv := range apiv3Paths {
6060
schema, err := getSchemaForPath(preferredApiGroups, path, gv)
6161
if err != nil {
62-
if b.log != nil {
63-
b.log.Debug().Err(err).Str("path", path).Msg("skipping schema path")
64-
}
62+
b.log.Debug().Err(err).Str("path", path).Msg("skipping schema path")
6563
continue
6664
}
6765
maps.Copy(b.schemas, schema)
@@ -100,26 +98,22 @@ func (b *SchemaBuilder) WithScope(rm meta.RESTMapper) *SchemaBuilder {
10098
}
10199

102100
if len(gvks) != 1 {
103-
if b.log != nil {
104-
b.log.Debug().Int("gvkCount", len(gvks)).Msg("skipping schema with unexpected GVK count")
105-
}
101+
b.log.Debug().Int("gvkCount", len(gvks)).Msg("skipping schema with unexpected GVK count")
106102
continue
107103
}
108104

109-
namespaced, err := apiutil.IsGVKNamespaced(k8sschema.GroupVersionKind{
105+
namespaced, err := apiutil.IsGVKNamespaced(runtimeSchema.GroupVersionKind{
110106
Group: gvks[0].Group,
111107
Version: gvks[0].Version,
112108
Kind: gvks[0].Kind,
113109
}, rm)
114110

115111
if err != nil {
116-
if b.log != nil {
117-
b.log.Debug().Err(err).
118-
Str("group", gvks[0].Group).
119-
Str("version", gvks[0].Version).
120-
Str("kind", gvks[0].Kind).
121-
Msg("failed to determine if GVK is namespaced")
122-
}
112+
b.log.Debug().Err(err).
113+
Str("group", gvks[0].Group).
114+
Str("version", gvks[0].Version).
115+
Str("kind", gvks[0].Kind).
116+
Msg("failed to determine if GVK is namespaced")
123117
continue
124118
}
125119

listener/pkg/apischema/crd_resolver.go

Lines changed: 24 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package apischema
33
import (
44
"encoding/json"
55
"errors"
6+
"fmt"
67
"slices"
78
"strings"
89

@@ -49,7 +50,7 @@ func (cr *CRDResolver) ResolveApiSchema(crd *apiextensionsv1.CustomResourceDefin
4950
return nil, errors.Join(ErrGetServerPreferred, err)
5051
}
5152

52-
preferredApiGroups, err := errorIfCRDNotInPreferredApiGroups(gkv, apiResLists, nil)
53+
preferredApiGroups, err := errorIfCRDNotInPreferredApiGroups(gkv, apiResLists, mustCreateLogger())
5354
if err != nil {
5455
return nil, errors.Join(ErrFilterPreferredResources, err)
5556
}
@@ -60,28 +61,33 @@ func (cr *CRDResolver) ResolveApiSchema(crd *apiextensionsv1.CustomResourceDefin
6061
Complete()
6162
}
6263

63-
func errorIfCRDNotInPreferredApiGroups(gkv *GroupKindVersions, apiResLists []*metav1.APIResourceList, log *logger.Logger) ([]string, error) {
64-
isKindFound := false
65-
preferredApiGroups := make([]string, 0, len(apiResLists))
66-
for _, apiResources := range apiResLists {
67-
gv, err := schema.ParseGroupVersion(apiResources.GroupVersion)
68-
if err != nil {
69-
if log != nil {
70-
log.Error().Err(err).Str("groupVersion", apiResources.GroupVersion).Msg("failed to parse group version")
64+
func errorIfCRDNotInPreferredApiGroups(gkv *GroupKindVersions, apiResLists []*metav1.APIResourceList, log logger.Logger) ([]string, error) {
65+
groupVersionStrings := make([]string, 0, len(gkv.Versions))
66+
for _, version := range gkv.Versions {
67+
gv := schema.GroupVersion{Group: gkv.Group, Version: version}
68+
gvString := gv.String()
69+
groupVersionStrings = append(groupVersionStrings, gvString)
70+
log.Debug().Str("groupVersion", gvString).Msg("checking if CRD group version is in preferred APIs")
71+
found := false
72+
for _, apiResList := range apiResLists {
73+
if apiResList.GroupVersion == gvString {
74+
found = true
75+
break
7176
}
72-
continue
7377
}
74-
isGroupFound := gkv.Group == gv.Group
75-
isVersionFound := slices.Contains(gkv.Versions, gv.Version)
76-
if isGroupFound && isVersionFound && !isKindFound {
77-
isKindFound = isCRDKindIncluded(gkv, apiResources)
78+
if !found {
79+
return groupVersionStrings, fmt.Errorf("CRD group version %s is not in preferred APIs", gvString)
7880
}
79-
preferredApiGroups = append(preferredApiGroups, apiResources.GroupVersion)
8081
}
81-
if !isKindFound {
82-
return nil, ErrGVKNotPreferred
82+
return groupVersionStrings, nil
83+
}
84+
85+
func mustCreateLogger() logger.Logger {
86+
logPtr, err := logger.New(logger.DefaultConfig())
87+
if err != nil {
88+
panic(fmt.Sprintf("failed to create default logger: %v", err))
8389
}
84-
return preferredApiGroups, nil
90+
return *logPtr
8591
}
8692

8793
func isCRDKindIncluded(gvk *GroupKindVersions, apiResources *metav1.APIResourceList) bool {
Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,20 @@
11
package clusteraccess
22

3-
// Integration testing exports for cross-package access
4-
// Unit tests within this package should use export_test.go instead
3+
import (
4+
"context"
55

6-
// ClusterAccessReconcilerPublic exposes the reconciler for integration testing
7-
type ClusterAccessReconcilerPublic = ClusterAccessReconciler
6+
"github.com/openmfp/golang-commons/logger"
7+
gatewayv1alpha1 "github.com/openmfp/kubernetes-graphql-gateway/common/apis/v1alpha1"
8+
)
89

9-
// GenerateSchemaSubroutinePublic exposes the subroutine for integration testing
10-
type GenerateSchemaSubroutinePublic = generateSchemaSubroutine
10+
// This file exports internal functions for integration testing
1111

12-
// NewGenerateSchemaSubroutineForTesting creates a new subroutine for integration testing
13-
func NewGenerateSchemaSubroutineForTesting(reconciler *ClusterAccessReconciler) *GenerateSchemaSubroutinePublic {
14-
return &generateSchemaSubroutine{reconciler: reconciler}
12+
// For integration tests that need access to schema generation functionality
13+
func GenerateSchemaForClusterAccessForTesting(
14+
ctx context.Context,
15+
clusterAccess gatewayv1alpha1.ClusterAccess,
16+
reconcilerInstance *ClusterAccessReconciler,
17+
log logger.Logger,
18+
) error {
19+
return generateSchemaForClusterAccess(ctx, clusterAccess, reconcilerInstance, log)
1520
}

listener/reconciler/clusteraccess/metadata_injector.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import (
1010
"github.com/openmfp/kubernetes-graphql-gateway/common/auth"
1111
)
1212

13-
func injectClusterMetadata(ctx context.Context, schemaJSON []byte, clusterAccess gatewayv1alpha1.ClusterAccess, k8sClient client.Client, log *logger.Logger) ([]byte, error) {
13+
func injectClusterMetadata(ctx context.Context, schemaJSON []byte, clusterAccess gatewayv1alpha1.ClusterAccess, k8sClient client.Client, log logger.Logger) ([]byte, error) {
1414
// Determine the path
1515
path := clusterAccess.Spec.Path
1616
if path == "" {
@@ -26,5 +26,5 @@ func injectClusterMetadata(ctx context.Context, schemaJSON []byte, clusterAccess
2626
}
2727

2828
// Use the common metadata injection function
29-
return auth.InjectClusterMetadata(ctx, schemaJSON, config, k8sClient, log)
29+
return auth.InjectClusterMetadata(ctx, schemaJSON, config, k8sClient, &log)
3030
}

0 commit comments

Comments
 (0)