Merge pull request #101 from vshn/fix/bucket-permissions

Deny bucket listing in iam policy

apis/exoscale/v1/conditions.go

@@ -0,0 +1,20 @@
+package v1
+
+import (
+	xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+// Updating returns a Ready condition where the service is updating.
+// Crossplane's runtine doesn't provide a pre-defined update condition for some
+// reason.
+func Updating() xpv1.Condition {
+	return xpv1.Condition{
+		Type:               xpv1.TypeReady,
+		Status:             corev1.ConditionFalse,
+		Reason:             "Updating",
+		Message:            "The service is being updated",
+		LastTransitionTime: metav1.Now(),
+	}
+}

operator/iamkeycontroller/create.go

@@ -69,45 +69,11 @@ func (p *IAMKeyPipeline) createIAMKey(ctx *pipelineContext) error {
 	log := controllerruntime.LoggerFrom(ctx)
 	log.Info("starting creation")
 
-	policyAllow := exooapi.IamServicePolicyRuleActionAllow
-	policyDeny := exooapi.IamServicePolicyRuleActionDeny
-	policyRules := exooapi.IamServicePolicyTypeRules
-
 	log.Info("IAM Role doesnt exists, creating", "keyName", ctx.iamKey.Spec.ForProvider.KeyName)
-	autogeneratedAppcatRole := &exooapi.IamRole{
-		Name:        &iamKey.Spec.ForProvider.KeyName,
-		Description: pointer.String("IAM Role for SOS+IAM creation, it was autogenerated by provider-exoscale"),
-		Permissions: &[]exooapi.IamRolePermissions{
-			exooapi.IamRolePermissionsBypassGovernanceRetention,
-		},
-		Editable: pointer.Bool(true),
-		Policy: &exooapi.IamPolicy{
-			DefaultServiceStrategy: exooapi.IamPolicyDefaultServiceStrategyDeny,
-			Services: exooapi.IamPolicy_Services{
-				AdditionalProperties: map[string]exooapi.IamServicePolicy{
-					"sos": {
-						Type:  &policyRules,
-						Rules: &[]exooapi.IamServicePolicyRule{},
-					},
-				},
-			},
-		},
-	}
-	// we must first add buckets to deny list and then add the allow rule, otherwise it will not work
-	for _, bucket := range iamKey.Spec.ForProvider.Services.SOS.Buckets {
-		*autogeneratedAppcatRole.Policy.Services.AdditionalProperties["sos"].Rules = append(*autogeneratedAppcatRole.Policy.Services.AdditionalProperties["sos"].Rules, exooapi.IamServicePolicyRule{
-			Action:     &policyDeny,
-			Expression: pointer.String("resources.bucket != " + "'" + bucket + "'"),
-		})
-	}
-
-	*autogeneratedAppcatRole.Policy.Services.AdditionalProperties["sos"].Rules = append(*autogeneratedAppcatRole.Policy.Services.AdditionalProperties["sos"].Rules, exooapi.IamServicePolicyRule{
-		Action:     &policyAllow,
-		Expression: pointer.String("true"),
-	})
+	autogeneratedAppcatRole := createRole(iamKey.Spec.ForProvider.KeyName, iamKey.Spec.ForProvider.Services.SOS.Buckets)
 
 	// send request
-	resp, err := ExecuteRequest(ctx, "POST", ctx.iamKey.Spec.ForProvider.Zone, "/v2/iam-role", p.apiKey, p.apiSecret, autogeneratedAppcatRole)
+	resp, err := executeRequest(ctx, "POST", ctx.iamKey.Spec.ForProvider.Zone, "/v2/iam-role", p.apiKey, p.apiSecret, autogeneratedAppcatRole)
 	if err != nil {
 		return err
 	}
@@ -133,7 +99,7 @@ func (p *IAMKeyPipeline) createIAMKey(ctx *pipelineContext) error {
 	// since their API is async and it needs a moment to create the IAM Role, we need to wait for it
 	for i := 0; i < 10; i++ {
 		// send request
-		resp, err = ExecuteRequest(ctx, "POST", ctx.iamKey.Spec.ForProvider.Zone, "/v2/api-key", p.apiKey, p.apiSecret, newIamKey)
+		resp, err = executeRequest(ctx, "POST", ctx.iamKey.Spec.ForProvider.Zone, "/v2/api-key", p.apiKey, p.apiSecret, newIamKey)
 		if err != nil {
 			time.Sleep(time.Millisecond * 500)
 			continue

operator/iamkeycontroller/delete.go

@@ -42,14 +42,14 @@ func (p *IAMKeyPipeline) deleteIAMKey(ctx *pipelineContext) error {
 
 	if iamKey.Status.AtProvider.RoleID != "" {
 
-		_, err := ExecuteRequest(ctx, "DELETE", ctx.iamKey.Spec.ForProvider.Zone, "/v2/api-key/"+iamKey.Status.AtProvider.KeyID, p.apiKey, p.apiSecret, nil)
+		_, err := executeRequest(ctx, "DELETE", ctx.iamKey.Spec.ForProvider.Zone, "/v2/api-key/"+iamKey.Status.AtProvider.KeyID, p.apiKey, p.apiSecret, nil)
 		if err != nil {
 			log.Error(err, "Cannot delete apiKey", "keyName", iamKey.Status.AtProvider.KeyID)
 			return err
 		}
 		log.Info("Iam key deleted successfully", "keyName", ctx.iamKey.Spec.ForProvider.KeyName)
 
-		_, err = ExecuteRequest(ctx, "DELETE", ctx.iamKey.Spec.ForProvider.Zone, "/v2/iam-role/"+iamKey.Status.AtProvider.RoleID, p.apiKey, p.apiSecret, nil)
+		_, err = executeRequest(ctx, "DELETE", ctx.iamKey.Spec.ForProvider.Zone, "/v2/iam-role/"+iamKey.Status.AtProvider.RoleID, p.apiKey, p.apiSecret, nil)
 		if err != nil {
 			log.Error(err, "Cannot delete iamRole", "iamrole", iamKey.Status.AtProvider.RoleID)
 			return err

operator/iamkeycontroller/observe.go

@@ -6,6 +6,7 @@ import (
 	"errors"
 	"fmt"
 	"net/url"
+	"reflect"
 
 	pipeline "github.com/ccremer/go-command-pipeline"
 	xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1"
@@ -78,6 +79,7 @@ func (p *IAMKeyPipeline) Observe(ctx context.Context, mg resource.Managed) (mana
 		WithSteps(
 			pipe.NewStep("fetch credentials secret", p.fetchCredentialsSecret),
 			pipe.NewStep("check credentials", p.checkSecret),
+			pipe.NewStep("check if role is up to date", p.isRoleUptodate),
 		).RunWithContext(pctx)
 
 	if err != nil {
@@ -108,7 +110,7 @@ func (p *IAMKeyPipeline) getIAMKey(ctx *pipelineContext) error {
 	keyDetails := exooapi.IamApiKey{}
 
 	// send request
-	resp, err := ExecuteRequest(ctx, "GET", ctx.iamKey.Spec.ForProvider.Zone, "/v2/api-key/"+ctx.iamKey.Status.AtProvider.KeyID, p.apiKey, p.apiSecret, nil)
+	resp, err := executeRequest(ctx, "GET", ctx.iamKey.Spec.ForProvider.Zone, "/v2/api-key/"+ctx.iamKey.Status.AtProvider.KeyID, p.apiKey, p.apiSecret, nil)
 	if err != nil {
 		return err
 	}
@@ -189,3 +191,46 @@ func isNotFound(err error) bool {
 	}
 	return false
 }
+
+func (p *IAMKeyPipeline) isRoleUptodate(ctx *pipelineContext) error {
+
+	// Only new keys support the roles. We don't handle legacy keys.
+	if _, exists := ctx.iamKey.Annotations["newKeyType"]; !exists {
+		return nil
+	}
+
+	errNotUpToDate := fmt.Errorf("roles are not equal, IAM Key is not up to date")
+
+	obsRole, err := p.observeRole(ctx)
+	if err != nil {
+		return errNotUpToDate
+	}
+
+	desiredRole := createRole(ctx.iamKey.Spec.ForProvider.KeyName, ctx.iamKey.Spec.ForProvider.Services.SOS.Buckets)
+
+	// We're only interested in the policy as most fields in the role can't be
+	// changed anyway after creation.
+	if !reflect.DeepEqual(obsRole.Policy, desiredRole.Policy) {
+		return errNotUpToDate
+	}
+
+	return nil
+}
+
+func (p *IAMKeyPipeline) observeRole(ctx *pipelineContext) (*exooapi.IamRole, error) {
+
+	resp, err := executeRequest(ctx, "GET", ctx.iamKey.Spec.ForProvider.Zone, "/v2/iam-role/"+ctx.iamKey.Status.AtProvider.RoleID, p.apiKey, p.apiSecret, nil)
+	if err != nil {
+		return nil, err
+	}
+
+	defer func() { _ = resp.Body.Close() }()
+
+	respRole := &exooapi.IamRole{}
+	err = json.NewDecoder(resp.Body).Decode(respRole)
+	if err != nil {
+		return nil, err
+	}
+
+	return respRole, nil
+}

operator/iamkeycontroller/pipeline.go

@@ -149,7 +149,7 @@ func signRequest(req *http.Request, expiration time.Time, apiKey, apiSecret stri
 	return nil
 }
 
-func ExecuteRequest(ctx context.Context, method, host, path, access_key, access_secret string, unMarshalledBody interface{}) (*http.Response, error) {
+func executeRequest(ctx context.Context, method, host, path, access_key, access_secret string, unMarshalledBody interface{}) (*http.Response, error) {
 	log := controllerruntime.LoggerFrom(ctx)
 	req := &http.Request{
 		Method: method,
@@ -173,7 +173,7 @@ func ExecuteRequest(ctx context.Context, method, host, path, access_key, access_
 		req.Body = io.NopCloser(bytes.NewReader(jsonbt))
 	}
 
-	if req.Method == "POST" {
+	if req.Method == "POST" || req.Method == "PUT" {
 		req.Header.Set("Content-Type", "application/json")
 	}
 

operator/iamkeycontroller/update.go

@@ -5,13 +5,22 @@ import (
 
 	"github.com/crossplane/crossplane-runtime/pkg/reconciler/managed"
 	"github.com/crossplane/crossplane-runtime/pkg/resource"
+	exov1 "github.com/vshn/provider-exoscale/apis/exoscale/v1"
 	controllerruntime "sigs.k8s.io/controller-runtime"
 )
 
 // Update implements managed.ExternalClient.
 // exoscale.com does not allow any updates on IAM keys.
 func (p *IAMKeyPipeline) Update(ctx context.Context, mg resource.Managed) (managed.ExternalUpdate, error) {
 	log := controllerruntime.LoggerFrom(ctx)
-	log.V(1).Info("Updating resource (noop)")
-	return managed.ExternalUpdate{}, nil
+	log.V(1).Info("Updating role")
+
+	iamKey := fromManaged(mg)
+	iamKey.SetConditions(exov1.Updating())
+
+	role := createRole(iamKey.Spec.ForProvider.KeyName, iamKey.Spec.ForProvider.Services.SOS.Buckets)
+
+	_, err := executeRequest(ctx, "PUT", iamKey.Spec.ForProvider.Zone, "/v2/iam-role/"+iamKey.Status.AtProvider.RoleID+":policy", p.apiKey, p.apiSecret, role.Policy)
+
+	return managed.ExternalUpdate{}, err
 }

operator/iamkeycontroller/util.go

@@ -0,0 +1,57 @@
+package iamkeycontroller
+
+import (
+	exooapi "github.com/exoscale/egoscale/v2/oapi"
+	"k8s.io/utils/pointer"
+)
+
+var (
+	policyAllow = exooapi.IamServicePolicyRuleActionAllow
+	policyDeny  = exooapi.IamServicePolicyRuleActionDeny
+)
+
+func createRole(keyName string, buckets []string) *exooapi.IamRole {
+
+	policyRules := exooapi.IamServicePolicyTypeRules
+
+	iamRole := &exooapi.IamRole{
+		Name:        &keyName,
+		Description: pointer.String("IAM Role for SOS+IAM creation, it was autogenerated by provider-exoscale"),
+		Permissions: &[]exooapi.IamRolePermissions{
+			exooapi.IamRolePermissionsBypassGovernanceRetention,
+		},
+		Editable: pointer.Bool(true),
+		Policy: &exooapi.IamPolicy{
+			DefaultServiceStrategy: exooapi.IamPolicyDefaultServiceStrategyDeny,
+			Services: exooapi.IamPolicy_Services{
+				AdditionalProperties: map[string]exooapi.IamServicePolicy{
+					"sos": {
+						Type:  &policyRules,
+						Rules: &[]exooapi.IamServicePolicyRule{},
+					},
+				},
+			},
+		},
+	}
+
+	// We specifically need to deny the listing of buckets, or the customer is able to see all of them
+	*iamRole.Policy.Services.AdditionalProperties["sos"].Rules = append(*iamRole.Policy.Services.AdditionalProperties["sos"].Rules, exooapi.IamServicePolicyRule{
+		Action:     &policyDeny,
+		Expression: pointer.String("operation in ['list-sos-buckets-usage', 'list-buckets']"),
+	})
+
+	// we must first add buckets to deny list and then add the allow rule, otherwise it will not work
+	for _, bucket := range buckets {
+		*iamRole.Policy.Services.AdditionalProperties["sos"].Rules = append(*iamRole.Policy.Services.AdditionalProperties["sos"].Rules, exooapi.IamServicePolicyRule{
+			Action:     &policyDeny,
+			Expression: pointer.String("resources.bucket != " + "'" + bucket + "'"),
+		})
+	}
+
+	*iamRole.Policy.Services.AdditionalProperties["sos"].Rules = append(*iamRole.Policy.Services.AdditionalProperties["sos"].Rules, exooapi.IamServicePolicyRule{
+		Action:     &policyAllow,
+		Expression: pointer.String("true"),
+	})
+
+	return iamRole
+}

test/e2e/kafka/00-assert.yaml

@@ -21,7 +21,7 @@ spec:
     size:
       plan: startup-2
     zone: ch-dk-2
-    version: '3.2'
+    version: '3.5'
   providerConfigRef:
     name: provider-config
   writeConnectionSecretToRef:

test/e2e/kafka/00-install.yaml

@@ -12,7 +12,7 @@ spec:
       timeOfDay: "12:00:00"
     size:
       plan: startup-2
-    version: '3.2'
+    version: '3.5'
     zone: ch-dk-2
   providerConfigRef:
     name: provider-config