Skip to content

Commit 527ed06

Browse files
shyamradhakrishnanshyamradhakrishnan
authored
Add doc for instance principal based e2e and fix empty subnet cidr bug (#93)
1 parent cbb0954 commit 527ed06

File tree

6 files changed

+173
-55
lines changed

6 files changed

+173
-55
lines changed

‎api/v1beta1/ocicluster_webhook_test.go‎

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,12 @@ func TestOCICluster_ValidateCreate(t *testing.T) {
4343
CIDR: "10.1.0.0/16",
4444
},
4545
}
46+
emptySubnetCidr := []*Subnet{
47+
&Subnet{
48+
Role: ControlPlaneRole,
49+
Name: "test-subnet",
50+
},
51+
}
4652
badSubnetCidrFormat := []*Subnet{
4753
&Subnet{
4854
Name: "test-subnet",
@@ -59,10 +65,11 @@ func TestOCICluster_ValidateCreate(t *testing.T) {
5965
CIDR: "10.0.0.0/16",
6066
},
6167
}
62-
badSubnetName := []*Subnet{
68+
emptySubnetName := []*Subnet{
6369
&Subnet{
6470
Name: "",
6571
CIDR: "10.0.0.0/16",
72+
Role: ControlPlaneEndpointRole,
6673
},
6774
}
6875
badSubnetRole := []*Subnet{
@@ -150,6 +157,23 @@ func TestOCICluster_ValidateCreate(t *testing.T) {
150157
errorMgsShouldContain: "cidr",
151158
expectErr: true,
152159
},
160+
{
161+
name: "should allow empty subnet cidr",
162+
c: &OCICluster{
163+
ObjectMeta: metav1.ObjectMeta{},
164+
Spec: OCIClusterSpec{
165+
CompartmentId: "ocid",
166+
OCIResourceIdentifier: "uuid",
167+
NetworkSpec: NetworkSpec{
168+
Vcn: VCN{
169+
CIDR: "10.0.0.0/16",
170+
Subnets: emptySubnetCidr,
171+
},
172+
},
173+
},
174+
},
175+
expectErr: false,
176+
},
153177
{
154178
name: "shouldn't allow subnet bad cidr format",
155179
c: &OCICluster{
@@ -199,20 +223,21 @@ func TestOCICluster_ValidateCreate(t *testing.T) {
199223
expectErr: true,
200224
},
201225
{
202-
name: "shouldn't allow invalid subnet name",
226+
name: "should allow empty subnet name",
203227
c: &OCICluster{
204228
ObjectMeta: metav1.ObjectMeta{},
205229
Spec: OCIClusterSpec{
230+
CompartmentId: "ocid",
231+
OCIResourceIdentifier: "uuid",
206232
NetworkSpec: NetworkSpec{
207233
Vcn: VCN{
208234
CIDR: "10.0.0.0/16",
209-
Subnets: badSubnetName,
235+
Subnets: emptySubnetName,
210236
},
211237
},
212238
},
213239
},
214-
errorMgsShouldContain: "subnet name invalid",
215-
expectErr: true,
240+
expectErr: false,
216241
},
217242
{
218243
name: "shouldn't allow bad NSG egress cidr",

‎api/v1beta1/validator.go‎

Lines changed: 30 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -97,28 +97,30 @@ func validateVCNCIDR(vncCIDR string, fldPath *field.Path) field.ErrorList {
9797
func validateSubnetCIDR(subnetCidr string, vcnCidr string, fldPath *field.Path) field.ErrorList {
9898
var allErrs field.ErrorList
9999

100-
subnetCidrIP, _, err := net.ParseCIDR(subnetCidr)
101-
if err != nil {
102-
allErrs = append(allErrs, field.Invalid(fldPath, subnetCidr, "invalid CIDR format"))
103-
}
104-
105-
// Check subnet is in vcnCidr if vcnCidr is set
106-
if len(vcnCidr) > 0 {
107-
var vcnNetwork *net.IPNet
108-
if _, parseNetwork, err := net.ParseCIDR(vcnCidr); err == nil {
109-
vcnNetwork = parseNetwork
100+
if len(subnetCidr) > 0 {
101+
subnetCidrIP, _, err := net.ParseCIDR(subnetCidr)
102+
if err != nil {
103+
allErrs = append(allErrs, field.Invalid(fldPath, subnetCidr, "invalid CIDR format"))
110104
}
111105

112-
var found bool
113-
if vcnNetwork != nil && vcnNetwork.Contains(subnetCidrIP) {
114-
found = true
115-
}
106+
// Check subnet is in vcnCidr if vcnCidr is set
107+
if len(vcnCidr) > 0 {
108+
var vcnNetwork *net.IPNet
109+
if _, parseNetwork, err := net.ParseCIDR(vcnCidr); err == nil {
110+
vcnNetwork = parseNetwork
111+
}
112+
113+
var found bool
114+
if vcnNetwork != nil && vcnNetwork.Contains(subnetCidrIP) {
115+
found = true
116+
}
116117

117-
if !found {
118-
allErrs = append(allErrs, field.Invalid(fldPath, subnetCidr, fmt.Sprintf("subnet CIDR not in VCN address space: %s", vcnCidr)))
118+
if !found {
119+
allErrs = append(allErrs, field.Invalid(fldPath, subnetCidr, fmt.Sprintf("subnet CIDR not in VCN address space: %s", vcnCidr)))
120+
}
119121
}
120-
}
121122

123+
}
122124
return allErrs
123125
}
124126

@@ -180,10 +182,12 @@ func validateSubnets(subnets []*Subnet, vcn VCN, fldPath *field.Path) field.Erro
180182
if err := validateSubnetName(subnet.Name, fldPath.Index(i).Child("name")); err != nil {
181183
allErrs = append(allErrs, err)
182184
}
183-
if _, ok := subnetNames[subnet.Name]; ok {
184-
allErrs = append(allErrs, field.Duplicate(fldPath, subnet.Name))
185+
if len(subnet.Name) > 0 {
186+
if _, ok := subnetNames[subnet.Name]; ok {
187+
allErrs = append(allErrs, field.Duplicate(fldPath, subnet.Name))
188+
}
189+
subnetNames[subnet.Name] = true
185190
}
186-
subnetNames[subnet.Name] = true
187191

188192
if err := validateRole(subnet.Role, fldPath.Index(i).Child("role"), "subnet role invalid"); err != nil {
189193
allErrs = append(allErrs, err)
@@ -197,9 +201,12 @@ func validateSubnets(subnets []*Subnet, vcn VCN, fldPath *field.Path) field.Erro
197201

198202
// validateSubnetName validates the Name of a Subnet.
199203
func validateSubnetName(name string, fldPath *field.Path) *field.Error {
200-
if invalidNameRegex.Match([]byte(name)) || name == "" {
201-
return field.Invalid(fldPath, name,
202-
fmt.Sprintf("subnet name invalid"))
204+
// subnet name can be empty
205+
if len(name) > 0 {
206+
if invalidNameRegex.Match([]byte(name)) || name == "" {
207+
return field.Invalid(fldPath, name,
208+
fmt.Sprintf("subnet name invalid"))
209+
}
203210
}
204211
return nil
205212
}

‎cloud/scope/subnet_reconciler.go‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -252,21 +252,26 @@ func (s *ClusterScope) GetSubnetsSpec() []*infrastructurev1beta1.Subnet {
252252
func (s *ClusterScope) SubnetSpec() ([]*infrastructurev1beta1.Subnet, error) {
253253
var subnets []*infrastructurev1beta1.Subnet
254254
var cidr string
255+
var name string
255256
var subnetType infrastructurev1beta1.SubnetType
256257
for _, subnet := range s.GetSubnetsSpec() {
257258
switch subnet.Role {
258259
case infrastructurev1beta1.ControlPlaneEndpointRole:
259260
subnetType = infrastructurev1beta1.Public
260261
cidr = ControlPlaneEndpointSubnetDefaultCIDR
262+
name = ControlPlaneEndpointDefaultName
261263
case infrastructurev1beta1.ControlPlaneRole:
262264
subnetType = infrastructurev1beta1.Private
263265
cidr = ControlPlaneMachineSubnetDefaultCIDR
266+
name = ControlPlaneDefaultName
264267
case infrastructurev1beta1.ServiceLoadBalancerRole:
265268
subnetType = infrastructurev1beta1.Public
266269
cidr = ServiceLoadBalancerDefaultCIDR
270+
name = ServiceLBDefaultName
267271
case infrastructurev1beta1.WorkerRole:
268272
subnetType = infrastructurev1beta1.Private
269273
cidr = WorkerSubnetDefaultCIDR
274+
name = WorkerDefaultName
270275
default:
271276
return nil, errors.New("invalid subnet role")
272277
}
@@ -276,9 +281,12 @@ func (s *ClusterScope) SubnetSpec() ([]*infrastructurev1beta1.Subnet, error) {
276281
if subnet.Type != "" {
277282
subnetType = subnet.Type
278283
}
284+
if subnet.Name != "" {
285+
name = subnet.Name
286+
}
279287
subnets = append(subnets, &infrastructurev1beta1.Subnet{
280288
Role: subnet.Role,
281-
Name: subnet.Name,
289+
Name: name,
282290
CIDR: cidr,
283291
Type: subnetType,
284292
SecurityList: subnet.SecurityList,

‎cloud/scope/subnet_reconciler_test.go‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -300,6 +300,56 @@ func TestClusterScope_SubnetSpec(t *testing.T) {
300300
},
301301
},
302302
},
303+
{
304+
name: "default names",
305+
wantErr: false,
306+
spec: infrastructurev1beta1.OCIClusterSpec{
307+
NetworkSpec: infrastructurev1beta1.NetworkSpec{
308+
Vcn: infrastructurev1beta1.VCN{
309+
Subnets: []*infrastructurev1beta1.Subnet{
310+
{
311+
Role: infrastructurev1beta1.ControlPlaneRole,
312+
},
313+
{
314+
Role: infrastructurev1beta1.WorkerRole,
315+
},
316+
{
317+
Role: infrastructurev1beta1.ServiceLoadBalancerRole,
318+
},
319+
{
320+
Role: infrastructurev1beta1.ControlPlaneEndpointRole,
321+
},
322+
},
323+
},
324+
},
325+
},
326+
want: []*infrastructurev1beta1.Subnet{
327+
{
328+
Role: infrastructurev1beta1.ControlPlaneEndpointRole,
329+
Name: ControlPlaneEndpointDefaultName,
330+
CIDR: ControlPlaneEndpointSubnetDefaultCIDR,
331+
Type: infrastructurev1beta1.Public,
332+
},
333+
{
334+
Role: infrastructurev1beta1.ControlPlaneRole,
335+
Name: ControlPlaneDefaultName,
336+
CIDR: ControlPlaneMachineSubnetDefaultCIDR,
337+
Type: infrastructurev1beta1.Private,
338+
},
339+
{
340+
Role: infrastructurev1beta1.WorkerRole,
341+
Name: WorkerDefaultName,
342+
CIDR: WorkerSubnetDefaultCIDR,
343+
Type: infrastructurev1beta1.Private,
344+
},
345+
{
346+
Role: infrastructurev1beta1.ServiceLoadBalancerRole,
347+
Name: ServiceLBDefaultName,
348+
CIDR: ServiceLoadBalancerDefaultCIDR,
349+
Type: infrastructurev1beta1.Public,
350+
},
351+
},
352+
},
303353
}
304354
l := log.FromContext(context.Background())
305355
for _, tt := range tests {

‎test/README.md‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
- Install `kustomize`
44
- If you have a running `kind` cluster, please delete the `kind` cluster
55

6-
# Export the following auth settings
6+
# Export the following auth settings if user principal is used as credential
77
```bash
88
export OCI_TENANCY_ID=<tenancy-id>
99
export OCI_USER_ID=<use-id>
@@ -17,10 +17,16 @@
1717
export OCI_REGION_B64="$(echo -n "$OCI_REGION" | base64 | tr -d '\n')"
1818
export OCI_CREDENTIALS_KEY_B64=$( cat <path-to-api-private-key-file> | base64 | tr -d '\n' )
1919
# if Passphrase is present
20-
export OCI_CREDENTIALS_PASSPHRASE_B64="$(echo -n "OCI_CREDENTIALS_PASSPHRASE" | base64 | tr -d '\n')"
20+
export OCI_CREDENTIALS_PASSPHRASE_B64="$(echo -n "$OCI_CREDENTIALS_PASSPHRASE" | base64 | tr -d '\n')"
21+
```
2122

23+
# Export the following auth settings if instance principal has to be used
24+
```bash
25+
export USE_INSTANCE_PRINCIPAL_B64="$(echo -n "true" | base64 | tr -d '\n')"
2226
```
2327

28+
29+
2430
# Export the following test settings
2531
```bash
2632
export OCI_COMPARTMENT_ID=<>

‎test/e2e/e2e_suite_test.go‎

Lines changed: 46 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"fmt"
2727
"os"
2828
"path/filepath"
29+
"strconv"
2930
"strings"
3031
"testing"
3132

@@ -183,34 +184,55 @@ var _ = SynchronizedBeforeSuite(func() []byte {
183184
e2eConfig = loadE2EConfig(configPath)
184185
bootstrapClusterProxy = NewOCIClusterProxy("bootstrap", kubeconfigPath, initScheme())
185186

186-
tenancyId, err := base64.StdEncoding.DecodeString(os.Getenv("OCI_TENANCY_ID_B64"))
187-
Expect(err).NotTo(HaveOccurred())
188-
userId, err := base64.StdEncoding.DecodeString(os.Getenv("OCI_USER_ID_B64"))
189-
Expect(err).NotTo(HaveOccurred())
190-
passphrase, err := base64.StdEncoding.DecodeString(os.Getenv("OCI_CREDENTIALS_PASSPHRASE_B64"))
191-
Expect(err).NotTo(HaveOccurred())
192-
key, err := base64.StdEncoding.DecodeString(os.Getenv("OCI_CREDENTIALS_KEY_B64"))
193-
Expect(err).NotTo(HaveOccurred())
194-
fingerprint, err := base64.StdEncoding.DecodeString(os.Getenv("OCI_CREDENTIALS_FINGERPRINT_B64"))
195-
Expect(err).NotTo(HaveOccurred())
196-
region, err := base64.StdEncoding.DecodeString(os.Getenv("OCI_REGION_B64"))
197-
Expect(err).NotTo(HaveOccurred())
198-
199-
ociAuthConfigProvider, err := oci_config.NewConfigurationProvider(&oci_config.AuthConfig{
200-
Region: string(region),
201-
TenancyID: string(tenancyId),
202-
UserID: string(userId),
203-
PrivateKey: string(key),
204-
Fingerprint: string(fingerprint),
205-
Passphrase: string(passphrase),
206-
UseInstancePrincipals: false,
207-
})
208-
Expect(err).NotTo(HaveOccurred())
187+
s, useInstancePrincipalFlagSet := os.LookupEnv("USE_INSTANCE_PRINCIPAL_B64")
188+
useInstanePrincipal := false
189+
if useInstancePrincipalFlagSet {
190+
useInstanePrincipalStr, err := base64.StdEncoding.DecodeString(s)
191+
Expect(err).NotTo(HaveOccurred())
192+
useInstanePrincipal, err = strconv.ParseBool(string(useInstanePrincipalStr))
193+
Expect(err).NotTo(HaveOccurred())
194+
}
195+
var ociAuthConfigProvider common.ConfigurationProvider
196+
var err error
197+
if useInstanePrincipal {
198+
ociAuthConfigProvider, err = oci_config.NewConfigurationProvider(&oci_config.AuthConfig{
199+
UseInstancePrincipals: true,
200+
})
201+
Expect(err).NotTo(HaveOccurred())
202+
By("Using instance principal as auth provider")
203+
} else {
204+
tenancyId, err := base64.StdEncoding.DecodeString(os.Getenv("OCI_TENANCY_ID_B64"))
205+
Expect(err).NotTo(HaveOccurred())
206+
userId, err := base64.StdEncoding.DecodeString(os.Getenv("OCI_USER_ID_B64"))
207+
Expect(err).NotTo(HaveOccurred())
208+
passphrase, err := base64.StdEncoding.DecodeString(os.Getenv("OCI_CREDENTIALS_PASSPHRASE_B64"))
209+
Expect(err).NotTo(HaveOccurred())
210+
key, err := base64.StdEncoding.DecodeString(os.Getenv("OCI_CREDENTIALS_KEY_B64"))
211+
Expect(err).NotTo(HaveOccurred())
212+
fingerprint, err := base64.StdEncoding.DecodeString(os.Getenv("OCI_CREDENTIALS_FINGERPRINT_B64"))
213+
Expect(err).NotTo(HaveOccurred())
214+
region, err := base64.StdEncoding.DecodeString(os.Getenv("OCI_REGION_B64"))
215+
Expect(err).NotTo(HaveOccurred())
216+
217+
ociAuthConfigProvider, err = oci_config.NewConfigurationProvider(&oci_config.AuthConfig{
218+
Region: string(region),
219+
TenancyID: string(tenancyId),
220+
UserID: string(userId),
221+
PrivateKey: string(key),
222+
Fingerprint: string(fingerprint),
223+
Passphrase: string(passphrase),
224+
UseInstancePrincipals: false,
225+
})
226+
Expect(err).NotTo(HaveOccurred())
227+
By("Using user principal as auth provider")
228+
}
209229

210230
clientProvider, err := scope.NewClientProvider(ociAuthConfigProvider)
211231
Expect(err).NotTo(HaveOccurred())
212232

213-
ociClients, err := clientProvider.GetOrBuildClient(string(region))
233+
region, err := ociAuthConfigProvider.Region()
234+
Expect(err).NotTo(HaveOccurred())
235+
ociClients, err := clientProvider.GetOrBuildClient(region)
214236
Expect(err).NotTo(HaveOccurred())
215237

216238
computeClient = ociClients.ComputeClient

0 commit comments

Comments
 (0)