Skip to content

Commit ee018ba

Browse files
vertex451vertex451
authored
fix: correct url for virtual ws (#170)
* fix: correct url for virtual ws On-behalf-of: @SAP [email protected] Signed-off-by: Artem Shcherbatiuk <[email protected]> * feat: made possible to run service in local dev mode without apiexport On-behalf-of: @SAP [email protected] Signed-off-by: Artem Shcherbatiuk <[email protected]>
1 parent 6c3f3e0 commit ee018ba

File tree

7 files changed

+174
-15
lines changed

7 files changed

+174
-15
lines changed

‎cmd/listener.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,11 @@ var listenCmd = &cobra.Command{
9191
Run: func(cmd *cobra.Command, args []string) {
9292
ctx := ctrl.SetupSignalHandler()
9393
restCfg := ctrl.GetConfigOrDie()
94+
log, err := setupLogger(appCfg.LogLevel)
95+
if err != nil {
96+
setupLog.Error(err, "unable to setup logger")
97+
os.Exit(1)
98+
}
9499

95100
mgrOpts := ctrl.Options{
96101
Scheme: scheme,
@@ -109,7 +114,7 @@ var listenCmd = &cobra.Command{
109114
os.Exit(1)
110115
}
111116

112-
mf := kcp.NewManagerFactory(appCfg)
117+
mf := kcp.NewManagerFactory(log, appCfg)
113118

114119
mgr, err := mf.NewManager(ctx, restCfg, mgrOpts, clt)
115120
if err != nil {
@@ -132,6 +137,7 @@ var listenCmd = &cobra.Command{
132137

133138
reconciler, err := kcp.NewReconciler(
134139
ctx,
140+
log,
135141
appCfg,
136142
reconcilerOpts,
137143
discoveryInterface,

‎listener/kcp/manager_factory.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ package kcp
33
import (
44
"context"
55
"errors"
6-
6+
"github.com/openmfp/golang-commons/logger"
77
"k8s.io/client-go/rest"
88
ctrl "sigs.k8s.io/controller-runtime"
99
"sigs.k8s.io/controller-runtime/pkg/client"
@@ -15,10 +15,12 @@ import (
1515

1616
type ManagerFactory struct {
1717
appConfig config.Config
18+
log *logger.Logger
1819
}
1920

20-
func NewManagerFactory(appCfg config.Config) *ManagerFactory {
21+
func NewManagerFactory(log *logger.Logger, appCfg config.Config) *ManagerFactory {
2122
return &ManagerFactory{
23+
log: log,
2224
appConfig: appCfg,
2325
}
2426
}
@@ -28,7 +30,7 @@ func (f *ManagerFactory) NewManager(ctx context.Context, restCfg *rest.Config, o
2830
return ctrl.NewManager(restCfg, opts)
2931
}
3032

31-
virtualWorkspaceCfg, err := virtualWorkspaceConfigFromCfg(ctx, f.appConfig, restCfg, clt)
33+
virtualWorkspaceCfg, err := virtualWorkspaceConfigFromCfg(ctx, f.log, f.appConfig, restCfg, clt)
3234
if err != nil {
3335
return nil, errors.Join(ErrGetVWConfig, err)
3436
}

‎listener/kcp/manager_factory_test.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@ package kcp
22

33
import (
44
"context"
5+
"github.com/openmfp/golang-commons/logger"
56
"github.com/openmfp/kubernetes-graphql-gateway/common/config"
7+
"github.com/stretchr/testify/require"
68
"testing"
79

810
kcpapis "github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha1"
@@ -25,6 +27,9 @@ func TestNewManager(t *testing.T) {
2527
"successful_manager_creation": {isKCPEnabled: false, expectErr: false},
2628
}
2729

30+
log, err := logger.New(logger.DefaultConfig())
31+
require.NoError(t, err)
32+
2833
for name, tc := range tests {
2934
scheme := runtime.NewScheme()
3035
err := kcpapis.AddToScheme(scheme)
@@ -47,11 +52,11 @@ func TestNewManager(t *testing.T) {
4752
},
4853
}...).Build()
4954

50-
f := NewManagerFactory(appCfg)
55+
f := NewManagerFactory(log, appCfg)
5156

5257
mgr, err := f.NewManager(
5358
context.Background(),
54-
&rest.Config{},
59+
&rest.Config{Host: validAPIServerHost},
5560
ctrl.Options{Scheme: scheme},
5661
fakeClient,
5762
)

‎listener/kcp/reconciler_factory.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package kcp
33
import (
44
"context"
55
"errors"
6+
"github.com/openmfp/golang-commons/logger"
67

78
"k8s.io/apimachinery/pkg/api/meta"
89
"k8s.io/apimachinery/pkg/runtime"
@@ -50,6 +51,7 @@ type ReconcilerOpts struct {
5051

5152
func NewReconciler(
5253
ctx context.Context,
54+
log *logger.Logger,
5355
appCfg config.Config,
5456
opts ReconcilerOpts,
5557
discoveryInterface discovery.DiscoveryInterface,
@@ -60,7 +62,7 @@ func NewReconciler(
6062
return newStandardReconciler(opts, discoveryInterface, preReconcileFunc)
6163
}
6264

63-
return newKcpReconciler(ctx, appCfg, opts, discoverFactory)
65+
return newKcpReconciler(ctx, log, appCfg, opts, discoverFactory)
6466
}
6567

6668
func newStandardReconciler(
@@ -120,6 +122,7 @@ func PreReconcile(
120122

121123
func newKcpReconciler(
122124
ctx context.Context,
125+
log *logger.Logger,
123126
appCfg config.Config,
124127
opts ReconcilerOpts,
125128
newDiscoveryFactoryFunc func(cfg *rest.Config) (*discoveryclient.Factory, error),
@@ -134,7 +137,7 @@ func newKcpReconciler(
134137
return nil, errors.Join(ErrCreatePathResolver, err)
135138
}
136139

137-
virtualWorkspaceCfg, err := virtualWorkspaceConfigFromCfg(ctx, appCfg, opts.Config, opts.Client)
140+
virtualWorkspaceCfg, err := virtualWorkspaceConfigFromCfg(ctx, log, appCfg, opts.Config, opts.Client)
138141
if err != nil {
139142
return nil, errors.Join(ErrGetVWConfig, err)
140143
}

‎listener/kcp/reconciler_factory_test.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,11 @@ package kcp
33
import (
44
"context"
55
"errors"
6+
"github.com/openmfp/golang-commons/logger"
67
"github.com/openmfp/kubernetes-graphql-gateway/common/config"
78
"github.com/openmfp/kubernetes-graphql-gateway/listener/clusterpath"
89
"github.com/openmfp/kubernetes-graphql-gateway/listener/kcp/mocks"
10+
"github.com/stretchr/testify/require"
911
"path"
1012
"testing"
1113

@@ -71,6 +73,9 @@ func TestNewReconciler(t *testing.T) {
7173
},
7274
}
7375

76+
log, err := logger.New(logger.DefaultConfig())
77+
require.NoError(t, err)
78+
7479
for name, tc := range tests {
7580
scheme := runtime.NewScheme()
7681
assert.NoError(t, kcpapis.AddToScheme(scheme))
@@ -96,6 +101,7 @@ func TestNewReconciler(t *testing.T) {
96101

97102
reconciler, err := NewReconciler(
98103
context.Background(),
104+
log,
99105
appCfg,
100106
ReconcilerOpts{
101107
Config: tc.cfg,

‎listener/kcp/workspace_config.go

Lines changed: 52 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,11 @@ package kcp
33
import (
44
"context"
55
"errors"
6+
"fmt"
7+
kcptenancy "github.com/kcp-dev/kcp/sdk/apis/tenancy/v1alpha1"
8+
"github.com/openmfp/golang-commons/logger"
9+
"net/url"
10+
"strings"
611
"time"
712

813
kcpapis "github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha1"
@@ -17,9 +22,16 @@ var (
1722
ErrFailedToGetAPIExport = errors.New("failed to get APIExport")
1823
ErrNoVirtualURLsFound = errors.New("no virtual URLs found for APIExport")
1924
ErrEmptyVirtualWorkspaceURL = errors.New("empty URL in virtual workspace for APIExport")
25+
ErrInvalidURL = errors.New("invalid URL format")
2026
)
2127

22-
func virtualWorkspaceConfigFromCfg(ctx context.Context, appCfg config.Config, restCfg *rest.Config, clt client.Client) (*rest.Config, error) {
28+
func virtualWorkspaceConfigFromCfg(
29+
ctx context.Context,
30+
log *logger.Logger,
31+
appCfg config.Config,
32+
restCfg *rest.Config,
33+
clt client.Client,
34+
) (*rest.Config, error) {
2335
timeOutDuration := 10 * time.Second
2436
ctx, cancelFn := context.WithTimeout(ctx, timeOutDuration)
2537
defer cancelFn()
@@ -30,10 +42,20 @@ func virtualWorkspaceConfigFromCfg(ctx context.Context, appCfg config.Config, re
3042
Name: appCfg.ApiExportName,
3143
}
3244
if err := clt.Get(ctx, key, &apiExport); err != nil {
33-
if errors.Is(ctx.Err(), context.DeadlineExceeded) {
34-
return nil, errors.Join(ErrTimeoutFetchingAPIExport, err)
45+
// if this is not a local development, we must have kubernetes.graphql.gateway apiexport
46+
if !appCfg.LocalDevelopment {
47+
if errors.Is(ctx.Err(), context.DeadlineExceeded) {
48+
return nil, errors.Join(ErrTimeoutFetchingAPIExport, err)
49+
}
50+
return nil, errors.Join(ErrFailedToGetAPIExport, err)
3551
}
36-
return nil, errors.Join(ErrFailedToGetAPIExport, err)
52+
53+
// otherwise fallback to the default APIExport, but live ApiBinding watching will not work
54+
if err = clt.Get(ctx, client.ObjectKey{Name: kcptenancy.SchemeGroupVersion.Group}, &apiExport); err != nil {
55+
return nil, errors.Join(ErrFailedToGetAPIExport, err)
56+
}
57+
58+
log.Warn().Msg(fmt.Sprintf("failed to find %s ApiExport, listener will not watch ApiBinding changes in realtime", appCfg.ApiExportName))
3759
}
3860

3961
if len(apiExport.Status.VirtualWorkspaces) == 0 { // nolint: staticcheck
@@ -45,7 +67,32 @@ func virtualWorkspaceConfigFromCfg(ctx context.Context, appCfg config.Config, re
4567
return nil, ErrEmptyVirtualWorkspaceURL
4668
}
4769

48-
restCfg.Host = virtualWorkspaceURL
70+
internalVirtualWorkspaceURL, err := combineBaseURLAndPath(restCfg.Host, virtualWorkspaceURL)
71+
if err != nil {
72+
return nil, err
73+
}
74+
75+
restCfg.Host = internalVirtualWorkspaceURL
4976

5077
return restCfg, nil
5178
}
79+
80+
func combineBaseURLAndPath(baseURLStr, pathURLStr string) (string, error) {
81+
baseURL, err := url.Parse(baseURLStr)
82+
if err != nil {
83+
return "", errors.Join(ErrInvalidURL, err)
84+
}
85+
86+
pathURL, err := url.Parse(pathURLStr)
87+
if err != nil {
88+
return "", errors.Join(ErrInvalidURL, err)
89+
}
90+
91+
path := pathURL.Path
92+
93+
if !strings.HasPrefix(path, "/") {
94+
path = "/" + path
95+
}
96+
97+
return baseURL.ResolveReference(&url.URL{Path: path}).String(), nil
98+
}

‎listener/kcp/workspace_config_test.go

Lines changed: 92 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@ package kcp
33
import (
44
"context"
55
"errors"
6+
"github.com/openmfp/golang-commons/logger"
67
"github.com/openmfp/kubernetes-graphql-gateway/common/config"
8+
"github.com/stretchr/testify/require"
79
"testing"
810

911
"github.com/stretchr/testify/assert"
@@ -42,7 +44,7 @@ func TestVirtualWorkspaceConfigFromCfg(t *testing.T) {
4244
},
4345
},
4446
"error_retrieving_APIExport": {
45-
err: errors.Join(ErrFailedToGetAPIExport, errors.New("apiexports.apis.kcp.io \"kubernetes.graphql.gateway\" not found")),
47+
err: errors.Join(ErrFailedToGetAPIExport, errors.New("apiexports.apis.kcp.io \"tenancy.kcp.io\" not found")),
4648
},
4749
"empty_virtual_workspace_list": {
4850
clientObjects: func(appCfg *config.Config) []client.Object {
@@ -75,22 +77,45 @@ func TestVirtualWorkspaceConfigFromCfg(t *testing.T) {
7577
},
7678
err: ErrEmptyVirtualWorkspaceURL,
7779
},
80+
"wrong_url_in_virtual_ws": {
81+
clientObjects: func(appCfg *config.Config) []client.Object {
82+
return []client.Object{
83+
&kcpapis.APIExport{
84+
ObjectMeta: metav1.ObjectMeta{
85+
Namespace: appCfg.ApiExportWorkspace,
86+
Name: appCfg.ApiExportName,
87+
},
88+
Status: kcpapis.APIExportStatus{
89+
VirtualWorkspaces: []kcpapis.VirtualWorkspace{
90+
{URL: "ht@tp://bad_url"},
91+
},
92+
},
93+
},
94+
}
95+
},
96+
err: errors.Join(ErrInvalidURL, errors.New("parse \"ht@tp://bad_url\": first path segment in URL cannot contain colon")),
97+
},
7898
}
7999

100+
log, err := logger.New(logger.DefaultConfig())
101+
require.NoError(t, err)
102+
80103
for name, tc := range tests {
81104
t.Run(name, func(t *testing.T) {
82105
appCfg, err := config.NewFromEnv()
83106
assert.NoError(t, err)
107+
appCfg.LocalDevelopment = true
84108

85109
fakeClientBuilder := fake.NewClientBuilder().WithScheme(scheme)
86110
if tc.clientObjects != nil {
87111
fakeClientBuilder.WithObjects(tc.clientObjects(&appCfg)...)
88112
}
89113
fakeClient := fakeClientBuilder.Build()
90114

91-
resultCfg, err := virtualWorkspaceConfigFromCfg(context.Background(), appCfg, &rest.Config{}, fakeClient)
115+
resultCfg, err := virtualWorkspaceConfigFromCfg(context.Background(), log, appCfg, &rest.Config{Host: validAPIServerHost}, fakeClient)
92116

93117
if tc.err != nil {
118+
// here it fails
94119
assert.EqualError(t, err, tc.err.Error())
95120
assert.Nil(t, resultCfg)
96121
} else {
@@ -100,3 +125,68 @@ func TestVirtualWorkspaceConfigFromCfg(t *testing.T) {
100125
})
101126
}
102127
}
128+
129+
func TestCombineBaseURLAndPath(t *testing.T) {
130+
tests := []struct {
131+
name string
132+
baseURL string
133+
pathURL string
134+
expected string
135+
err error
136+
}{
137+
{
138+
name: "success",
139+
baseURL: "https://openmfp-kcp-front-proxy.openmfp-system:8443/clusters/root",
140+
pathURL: "https://kcp.dev.local:8443/services/apiexport/root/kubernetes.graphql.gateway",
141+
expected: "https://openmfp-kcp-front-proxy.openmfp-system:8443/services/apiexport/root/kubernetes.graphql.gateway",
142+
},
143+
{
144+
name: "success_base_with_port",
145+
baseURL: "https://example.com:8080",
146+
pathURL: "/api/resource",
147+
expected: "https://example.com:8080/api/resource",
148+
},
149+
{
150+
name: "success_base_with_subpath_relative_path",
151+
baseURL: "https://example.com/base",
152+
pathURL: "api/resource",
153+
expected: "https://example.com/api/resource",
154+
},
155+
{
156+
name: "success_base_with_subpath_absolute_path",
157+
baseURL: "https://example.com/base",
158+
pathURL: "/api/resource",
159+
expected: "https://example.com/api/resource",
160+
},
161+
{
162+
name: "success_empty_path_url",
163+
baseURL: "https://example.com",
164+
pathURL: "",
165+
expected: "https://example.com/",
166+
},
167+
{
168+
name: "error_invalid_base_url",
169+
baseURL: "ht@tp://bad_url",
170+
pathURL: "/api/resource",
171+
err: errors.Join(ErrInvalidURL, errors.New("parse \"ht@tp://bad_url\": first path segment in URL cannot contain colon")),
172+
},
173+
{
174+
name: "error_invalid_path_url",
175+
baseURL: "https://example.com",
176+
pathURL: "ht@tp://bad_url",
177+
err: errors.Join(ErrInvalidURL, errors.New("parse \"ht@tp://bad_url\": first path segment in URL cannot contain colon")),
178+
},
179+
}
180+
181+
for _, tt := range tests {
182+
t.Run(tt.name, func(t *testing.T) {
183+
result, err := combineBaseURLAndPath(tt.baseURL, tt.pathURL)
184+
185+
if tt.err != nil {
186+
assert.EqualError(t, err, tt.err.Error())
187+
} else {
188+
assert.Equal(t, tt.expected, result)
189+
}
190+
})
191+
}
192+
}

0 commit comments

Comments
 (0)