Skip to content

Commit 6f78e5f

Browse files
barbacbdbarbacbd
committed
** Added tests for compute -> firewalls, subnets, network
1 parent f5001f5 commit 6f78e5f

File tree

7 files changed

+768
-8
lines changed

7 files changed

+768
-8
lines changed

‎cloud/scope/machine.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,7 @@ func (m *MachineScope) InstanceImageSpec() *compute.AttachedDisk {
228228
version = *m.Machine.Spec.Version
229229
}
230230
image := "capi-ubuntu-1804-k8s-" + strings.ReplaceAll(semver.MajorMinor(version), ".", "-")
231-
sourceImage := path.Join("projects", m.ClusterGetter.Project(), "global", "images", "family", image)
231+
sourceImage := path.Join("projects", m.ClusterGetter.NetworkProject(), "global", "images", "family", image)
232232
if m.GCPMachine.Spec.Image != nil {
233233
sourceImage = *m.GCPMachine.Spec.Image
234234
} else if m.GCPMachine.Spec.ImageFamily != nil {

‎cloud/services/compute/firewalls/reconcile.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ import (
2828
func (s *Service) Reconcile(ctx context.Context) error {
2929
log := log.FromContext(ctx)
3030
if s.scope.IsSharedVpc() {
31-
log.Info("VPC enabled. Ignore Reconciling firewall resources")
31+
log.Info("Shared VPC enabled. Ignore Reconciling firewall resources")
3232
return nil
3333
}
3434
log.Info("Reconciling firewall resources")
@@ -54,7 +54,7 @@ func (s *Service) Reconcile(ctx context.Context) error {
5454
func (s *Service) Delete(ctx context.Context) error {
5555
log := log.FromContext(ctx)
5656
if s.scope.IsSharedVpc() {
57-
log.Info("VPC enabled. Ignore Deleting firewall resources")
57+
log.Info("Shared VPC enabled. Ignore Deleting firewall resources")
5858
return nil
5959
}
6060
log.Info("Deleting firewall resources")

‎cloud/services/compute/firewalls/reconcile_test.go‎

Lines changed: 289 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,289 @@
1+
/*
2+
Copyright 2024 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package firewalls
18+
19+
import (
20+
"context"
21+
"fmt"
22+
"net/http"
23+
"testing"
24+
25+
"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud"
26+
"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta"
27+
"google.golang.org/api/compute/v1"
28+
"google.golang.org/api/googleapi"
29+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
30+
"k8s.io/client-go/kubernetes/scheme"
31+
"k8s.io/utils/ptr"
32+
infrav1 "sigs.k8s.io/cluster-api-provider-gcp/api/v1beta1"
33+
"sigs.k8s.io/cluster-api-provider-gcp/cloud/scope"
34+
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
35+
"sigs.k8s.io/controller-runtime/pkg/client/fake"
36+
)
37+
38+
func init() {
39+
_ = clusterv1.AddToScheme(scheme.Scheme)
40+
_ = infrav1.AddToScheme(scheme.Scheme)
41+
}
42+
43+
var fakeCluster = &clusterv1.Cluster{
44+
ObjectMeta: metav1.ObjectMeta{
45+
Name: "my-cluster",
46+
Namespace: "default",
47+
},
48+
Spec: clusterv1.ClusterSpec{},
49+
}
50+
51+
var fakeGCPCluster = &infrav1.GCPCluster{
52+
ObjectMeta: metav1.ObjectMeta{
53+
Name: "my-cluster",
54+
Namespace: "default",
55+
},
56+
Spec: infrav1.GCPClusterSpec{
57+
Project: "my-proj",
58+
Region: "us-central1",
59+
Network: infrav1.NetworkSpec{
60+
Name: ptr.To("my-network"),
61+
Subnets: infrav1.Subnets{
62+
infrav1.SubnetSpec{
63+
Name: "workers",
64+
CidrBlock: "10.0.0.1/28",
65+
Region: "us-central1",
66+
Purpose: ptr.To[string]("INTERNAL_HTTPS_LOAD_BALANCER"),
67+
},
68+
},
69+
},
70+
},
71+
Status: infrav1.GCPClusterStatus{
72+
Network: infrav1.Network{
73+
FirewallRules: map[string]string{
74+
fmt.Sprintf("allow-%s-healthchecks", "my-network"): "test",
75+
},
76+
},
77+
},
78+
}
79+
80+
var fakeGCPClusterSharedVPC = &infrav1.GCPCluster{
81+
ObjectMeta: metav1.ObjectMeta{
82+
Name: "my-cluster",
83+
Namespace: "default",
84+
},
85+
Spec: infrav1.GCPClusterSpec{
86+
Project: "my-proj",
87+
Region: "us-central1",
88+
Network: infrav1.NetworkSpec{
89+
HostProject: ptr.To("my-shared-vpc-project"),
90+
Name: ptr.To("my-network"),
91+
Subnets: infrav1.Subnets{
92+
infrav1.SubnetSpec{
93+
Name: "workers",
94+
CidrBlock: "10.0.0.1/28",
95+
Region: "us-central1",
96+
Purpose: ptr.To[string]("INTERNAL_HTTPS_LOAD_BALANCER"),
97+
},
98+
},
99+
},
100+
},
101+
Status: infrav1.GCPClusterStatus{
102+
Network: infrav1.Network{
103+
FirewallRules: map[string]string{
104+
"my-cluster-apiserver": "test",
105+
"my-cluster-apiintserver": "test",
106+
},
107+
},
108+
},
109+
}
110+
111+
type testCase struct {
112+
name string
113+
scope func() Scope
114+
mockFirewalls *cloud.MockFirewalls
115+
wantErr bool
116+
assert func(ctx context.Context, t testCase) error
117+
}
118+
119+
func TestService_Reconcile(t *testing.T) {
120+
fakec := fake.NewClientBuilder().
121+
WithScheme(scheme.Scheme).
122+
Build()
123+
124+
clusterScope, err := scope.NewClusterScope(context.TODO(), scope.ClusterScopeParams{
125+
Client: fakec,
126+
Cluster: fakeCluster,
127+
GCPCluster: fakeGCPCluster,
128+
GCPServices: scope.GCPServices{
129+
Compute: &compute.Service{},
130+
},
131+
})
132+
if err != nil {
133+
t.Fatal(err)
134+
}
135+
136+
clusterScopeSharedVpc, err := scope.NewClusterScope(context.TODO(), scope.ClusterScopeParams{
137+
Client: fakec,
138+
Cluster: fakeCluster,
139+
GCPCluster: fakeGCPClusterSharedVPC,
140+
GCPServices: scope.GCPServices{
141+
Compute: &compute.Service{},
142+
},
143+
})
144+
if err != nil {
145+
t.Fatal(err)
146+
}
147+
148+
tests := []testCase{
149+
{
150+
name: "firewall rule already exist (should return existing firewall rule)",
151+
scope: func() Scope { return clusterScope },
152+
mockFirewalls: &cloud.MockFirewalls{
153+
ProjectRouter: &cloud.SingleProjectRouter{ID: "my-proj"},
154+
Objects: map[meta.Key]*cloud.MockFirewallsObj{
155+
*meta.GlobalKey(fmt.Sprintf("allow-%s-healthchecks", fakeGCPCluster.ObjectMeta.Name)): {},
156+
},
157+
},
158+
},
159+
{
160+
name: "error getting instance with non 404 error code (should return an error)",
161+
scope: func() Scope { return clusterScope },
162+
mockFirewalls: &cloud.MockFirewalls{
163+
ProjectRouter: &cloud.SingleProjectRouter{ID: "my-proj"},
164+
Objects: map[meta.Key]*cloud.MockFirewallsObj{},
165+
GetHook: func(ctx context.Context, key *meta.Key, m *cloud.MockFirewalls) (bool, *compute.Firewall, error) {
166+
return true, &compute.Firewall{}, &googleapi.Error{Code: http.StatusBadRequest}
167+
},
168+
},
169+
wantErr: true,
170+
},
171+
{
172+
name: "firewall rule creation fails (should return an error)",
173+
scope: func() Scope { return clusterScope },
174+
mockFirewalls: &cloud.MockFirewalls{
175+
ProjectRouter: &cloud.SingleProjectRouter{ID: "my-proj"},
176+
Objects: map[meta.Key]*cloud.MockFirewallsObj{},
177+
InsertError: map[meta.Key]error{
178+
*meta.GlobalKey(fmt.Sprintf("allow-%s-healthchecks", fakeGCPCluster.ObjectMeta.Name)): &googleapi.Error{Code: http.StatusBadRequest},
179+
},
180+
},
181+
wantErr: true,
182+
},
183+
{
184+
name: "firewall return no error using shared vpc",
185+
scope: func() Scope { return clusterScopeSharedVpc },
186+
mockFirewalls: &cloud.MockFirewalls{
187+
ProjectRouter: &cloud.SingleProjectRouter{ID: "my-proj"},
188+
Objects: map[meta.Key]*cloud.MockFirewallsObj{
189+
*meta.GlobalKey(fmt.Sprintf("allow-%s-healthchecks", fakeGCPCluster.ObjectMeta.Name)): {},
190+
},
191+
},
192+
},
193+
}
194+
for _, tt := range tests {
195+
t.Run(tt.name, func(t *testing.T) {
196+
ctx := context.TODO()
197+
s := New(tt.scope())
198+
s.firewalls = tt.mockFirewalls
199+
err := s.Reconcile(ctx)
200+
if (err != nil) != tt.wantErr {
201+
t.Errorf("Service.Reconcile() error = %v, wantErr %v", err, tt.wantErr)
202+
return
203+
}
204+
if tt.assert != nil {
205+
err = tt.assert(ctx, tt)
206+
if err != nil {
207+
t.Errorf("firewall rule was not created as expected: %v", err)
208+
return
209+
}
210+
}
211+
})
212+
}
213+
}
214+
215+
func TestService_Delete(t *testing.T) {
216+
fakec := fake.NewClientBuilder().
217+
WithScheme(scheme.Scheme).
218+
Build()
219+
220+
clusterScope, err := scope.NewClusterScope(context.TODO(), scope.ClusterScopeParams{
221+
Client: fakec,
222+
Cluster: fakeCluster,
223+
GCPCluster: fakeGCPCluster,
224+
GCPServices: scope.GCPServices{
225+
Compute: &compute.Service{},
226+
},
227+
})
228+
if err != nil {
229+
t.Fatal(err)
230+
}
231+
232+
clusterScopeSharedVpc, err := scope.NewClusterScope(context.TODO(), scope.ClusterScopeParams{
233+
Client: fakec,
234+
Cluster: fakeCluster,
235+
GCPCluster: fakeGCPClusterSharedVPC,
236+
GCPServices: scope.GCPServices{
237+
Compute: &compute.Service{},
238+
},
239+
})
240+
if err != nil {
241+
t.Fatal(err)
242+
}
243+
244+
tests := []testCase{
245+
{
246+
name: "firewall rule does not exist, should do nothing",
247+
scope: func() Scope { return clusterScope },
248+
mockFirewalls: &cloud.MockFirewalls{
249+
ProjectRouter: &cloud.SingleProjectRouter{ID: "my-proj"},
250+
DeleteError: map[meta.Key]error{
251+
*meta.GlobalKey(fmt.Sprintf("allow-%s-healthchecks", fakeGCPCluster.ObjectMeta.Name)): &googleapi.Error{Code: http.StatusNotFound},
252+
},
253+
},
254+
},
255+
{
256+
name: "error deleting firewall rule, should return error",
257+
scope: func() Scope { return clusterScope },
258+
mockFirewalls: &cloud.MockFirewalls{
259+
ProjectRouter: &cloud.SingleProjectRouter{ID: "my-proj"},
260+
DeleteError: map[meta.Key]error{
261+
*meta.GlobalKey(fmt.Sprintf("allow-%s-healthchecks", fakeGCPCluster.ObjectMeta.Name)): &googleapi.Error{Code: http.StatusBadRequest},
262+
},
263+
},
264+
wantErr: true,
265+
},
266+
{
267+
name: "firewall rule deletion with shared vpc",
268+
scope: func() Scope { return clusterScopeSharedVpc },
269+
mockFirewalls: &cloud.MockFirewalls{
270+
ProjectRouter: &cloud.SingleProjectRouter{ID: "my-proj"},
271+
DeleteError: map[meta.Key]error{
272+
*meta.GlobalKey(fmt.Sprintf("allow-%s-healthchecks", *fakeGCPCluster.Spec.Network.Name)): &googleapi.Error{Code: http.StatusNotFound},
273+
},
274+
},
275+
},
276+
}
277+
for _, tt := range tests {
278+
t.Run(tt.name, func(t *testing.T) {
279+
ctx := context.TODO()
280+
s := New(tt.scope())
281+
s.firewalls = tt.mockFirewalls
282+
err := s.Delete(ctx)
283+
if (err != nil) != tt.wantErr {
284+
t.Errorf("Service.Delete() error = %v, wantErr %v", err, tt.wantErr)
285+
return
286+
}
287+
})
288+
}
289+
}

‎cloud/services/compute/networks/reconcile.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ func (s *Service) Reconcile(ctx context.Context) error {
5353
func (s *Service) Delete(ctx context.Context) error {
5454
log := log.FromContext(ctx)
5555
if s.scope.IsSharedVpc() {
56-
log.Info("VPC enabled. Ignore Deleting network resources")
56+
log.Info("Shared VPC enabled. Ignore Deleting network resources")
5757
s.scope.Network().Router = nil
5858
s.scope.Network().SelfLink = nil
5959
return nil
@@ -109,7 +109,7 @@ func (s *Service) createOrGetNetwork(ctx context.Context) (*compute.Network, err
109109
}
110110

111111
if s.scope.IsSharedVpc() {
112-
log.Error(err, "VPC is enabled. Error looking for network", "name", s.scope.NetworkName())
112+
log.Error(err, "Shared VPC is enabled. Error looking for network", "name", s.scope.NetworkName())
113113
return nil, err
114114
}
115115

0 commit comments

Comments
 (0)