Migration 8 to 16 scratch pad

api.go

@@ -31,10 +31,10 @@ import (
 func APICommand() *cobra.Command {
 	roles := []string{}
 	usernamePrefix := ""
-	var allowEmptyBillingEntity bool
+	var allowEmptyBillingEntity, skipBillingEntityValidation bool
 
 	ob := &odooStorageBuilder{}
-	ost := orgStore.New(&roles, &usernamePrefix, &allowEmptyBillingEntity)
+	ost := orgStore.New(&roles, &usernamePrefix, &allowEmptyBillingEntity, &skipBillingEntityValidation)
 	ib := &invitationStorageBuilder{usernamePrefix: &usernamePrefix}
 
 	cmd, err := builder.APIServer.
@@ -55,6 +55,7 @@ func APICommand() *cobra.Command {
 	cmd.Flags().StringSliceVar(&roles, "cluster-roles", []string{}, "Cluster Roles to bind when creating an organization")
 	cmd.Flags().StringVar(&usernamePrefix, "username-prefix", "", "Prefix prepended to username claims. Usually the same as \"--oidc-username-prefix\" of the Kubernetes API server")
 	cmd.Flags().BoolVar(&allowEmptyBillingEntity, "allow-empty-billing-entity", true, "Allow empty billing entity references")
+	cmd.Flags().BoolVar(&skipBillingEntityValidation, "organization-skip-billing-entity-validation", false, "Skip validation of billing entity references")
 
 	cmd.Flags().StringVar(&ob.billingEntityStorage, "billing-entity-storage", "fake", "Storage backend for billing entities. Supported values: fake, odoo8, odoo16")
 

apiserver/billing/odoostorage/odoo/odoo16/odoo16.go

@@ -23,15 +23,16 @@ import (
 const VSHNAccountingContactNameKey = "billing.appuio.io/vshn-accounting-contact-name"
 
 // Used to identify the accounting contact of a company.
-const roleAccountCategory = 70
-const companyCategory = 2
 const invoiceType = "invoice"
 
+// TODO(bastjan) test if still needed in odoo16
+const companyCategory = 2
+
 // Used to generate the UUID for the .metadata.uid field.
 var metaUIDNamespace = uuid.MustParse("7550b1ae-7a2a-485e-a75d-6f931b2cd73f")
 
-var roleAccountFilter = odooclient.NewCriterion("category_id", "in", []int{roleAccountCategory})
 var activeFilter = odooclient.NewCriterion("active", "=", true)
+var invoiceTypeFilter = odooclient.NewCriterion("type", "=", invoiceType)
 var notInflightFilter = odooclient.NewCriterion("vshn_control_api_inflight", "=", false)
 var mustInflightFilter = odooclient.NewCriterion("vshn_control_api_inflight", "!=", false)
 
@@ -97,8 +98,8 @@ type FailedRecordScrubber struct {
 	sessionCreator func(ctx context.Context) (Odoo16Client, error)
 }
 
+//go:generate go run go.uber.org/mock/mockgen -destination=./odoo16mock/$GOFILE -package odoo16mock . Odoo16Client
 type Odoo16Client interface {
-	Read(string, []int64, *odooclient.Options, interface{}) error
 	Update(string, []int64, interface{}) error
 	FindResPartners(*odooclient.Criteria, *odooclient.Options) (*odooclient.ResPartners, error)
 	CreateResPartner(*odooclient.ResPartner) (int64, error)
@@ -127,28 +128,45 @@ func (s *Odoo16Storage) get(ctx context.Context, name string) (company odooclien
 		return odooclient.ResPartner{}, odooclient.ResPartner{}, err
 	}
 
-	u := []odooclient.ResPartner{}
-	err = session.Read(odooclient.ResPartnerModel, []int64{int64(id)}, fetchPartnerFieldOpts, &u)
+	accp, err := session.FindResPartners(
+		newValidInvoiceRecordCriteria().AddCriterion(odooclient.NewCriterion("id", "=", id)),
+		fetchPartnerFieldOpts)
 	if err != nil {
-		return odooclient.ResPartner{}, odooclient.ResPartner{}, fmt.Errorf("fetching accounting contact by ID: %w", err)
+		return odooclient.ResPartner{}, odooclient.ResPartner{}, fmt.Errorf("error fetching accounting contact %d: %w", id, err)
+	}
+	if accp == nil {
+		return odooclient.ResPartner{}, odooclient.ResPartner{}, fmt.Errorf("fetching accounting contact %d returned nil", id)
+	}
+	acc := *accp
+	if len(acc) <= 0 {
+		return odooclient.ResPartner{}, odooclient.ResPartner{}, fmt.Errorf("no results when fetching accounting contact %d", id)
 	}
-	if len(u) <= 0 {
-		return odooclient.ResPartner{}, odooclient.ResPartner{}, fmt.Errorf("no results when fetching accounting contact by ID")
+	if len(acc) > 1 {
+		return odooclient.ResPartner{}, odooclient.ResPartner{}, fmt.Errorf("multiple results when fetching accounting contact %d", id)
 	}
-	accountingContact = u[0]
+	accountingContact = acc[0]
 
 	if accountingContact.ParentId == nil {
 		return odooclient.ResPartner{}, odooclient.ResPartner{}, fmt.Errorf("accounting contact %d has no parent", id)
 	}
 
-	err = session.Read(odooclient.ResPartnerModel, []int64{accountingContact.ParentId.ID}, fetchPartnerFieldOpts, &u)
+	cpp, err := session.FindResPartners(
+		odooclient.NewCriteria().AddCriterion(activeFilter).AddCriterion(odooclient.NewCriterion("id", "=", id)),
+		fetchPartnerFieldOpts)
 	if err != nil {
 		return odooclient.ResPartner{}, odooclient.ResPartner{}, fmt.Errorf("fetching parent %d of accounting contact %d failed: %w", accountingContact.ParentId.ID, id, err)
 	}
-	if len(u) <= 0 {
-		return odooclient.ResPartner{}, odooclient.ResPartner{}, fmt.Errorf("no results when fetching parent %d of accounting contact %d failed", accountingContact.ParentId.ID, id)
+	if cpp == nil {
+		return odooclient.ResPartner{}, odooclient.ResPartner{}, fmt.Errorf("fetching parent %d of accounting contact %d returned nil", accountingContact.ParentId.ID, id)
 	}
-	company = u[0]
+	cp := *cpp
+	if len(cp) <= 0 {
+		return odooclient.ResPartner{}, odooclient.ResPartner{}, fmt.Errorf("no results when fetching parent %d of accounting contact %d", accountingContact.ParentId.ID, id)
+	}
+	if len(cp) > 1 {
+		return odooclient.ResPartner{}, odooclient.ResPartner{}, fmt.Errorf("multiple results when fetching parent %d of accounting contact %d", accountingContact.ParentId.ID, id)
+	}
+	company = cp[0]
 
 	return company, accountingContact, nil
 }
@@ -161,8 +179,7 @@ func (s *Odoo16Storage) List(ctx context.Context) ([]billingv1.BillingEntity, er
 		return nil, err
 	}
 
-	criteria := odooclient.NewCriteria().AddCriterion(roleAccountFilter).AddCriterion(activeFilter).AddCriterion(notInflightFilter)
-	accPartners, err := session.FindResPartners(criteria, fetchPartnerFieldOpts)
+	accPartners, err := session.FindResPartners(newValidInvoiceRecordCriteria(), fetchPartnerFieldOpts)
 	if err != nil {
 		return nil, err
 	}
@@ -176,7 +193,7 @@ func (s *Odoo16Storage) List(ctx context.Context) ([]billingv1.BillingEntity, er
 		companyIDs = append(companyIDs, int(p.ParentId.ID))
 	}
 
-	criteria = odooclient.NewCriteria().AddCriterion(activeFilter).AddCriterion(odooclient.NewCriterion("id", "in", companyIDs))
+	criteria := odooclient.NewCriteria().AddCriterion(activeFilter).AddCriterion(odooclient.NewCriterion("id", "in", companyIDs))
 	companies, err := session.FindResPartners(criteria, fetchPartnerFieldOpts)
 	if err != nil {
 		return nil, err
@@ -426,7 +443,6 @@ func mapBillingEntityToPartners(be billingv1.BillingEntity, countryIDs map[strin
 
 func setStaticAccountingContactFields(conf Config, a *odooclient.ResPartner) {
 	a.CategoryId = odooclient.NewRelation()
-	a.CategoryId.AddRecord(int64(roleAccountCategory))
 	a.Lang = odooclient.NewSelection(conf.LanguagePreference)
 	a.Type = odooclient.NewSelection(invoiceType)
 	a.PropertyPaymentTermId = odooclient.NewMany2One(int64(conf.PaymentTermID), "")
@@ -449,3 +465,10 @@ func splitCommaSeparated(s string) []string {
 	}
 	return p
 }
+
+func newValidInvoiceRecordCriteria() *odooclient.Criteria {
+	return odooclient.NewCriteria().
+		AddCriterion(invoiceTypeFilter).
+		AddCriterion(activeFilter).
+		AddCriterion(notInflightFilter)
+}

apiserver/billing/odoostorage/odoo/odoo16/odoo16_test.go

@@ -25,17 +25,17 @@ func TestGet(t *testing.T) {
 	statusTime := st.Local()
 
 	gomock.InOrder(
-		mock.EXPECT().Read(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).SetArg(3, []odooclient.ResPartner{{
+		mock.EXPECT().FindResPartners(gomock.Any(), gomock.Any()).Return(&odooclient.ResPartners{{
 			Id:                       odooclient.NewInt(456),
 			CreateDate:               odooclient.NewTime(tn),
 			ParentId:                 odooclient.NewMany2One(123, ""),
 			Email:                    odooclient.NewString("[email protected], [email protected]"),
 			VshnControlApiMetaStatus: odooclient.NewString("{\"conditions\":[{\"type\":\"ConditionFoo\",\"status\":\"False\",\"lastTransitionTime\":\"" + statusTime.Format(time.RFC3339) + "\",\"reason\":\"Whatever\",\"message\":\"Hello World\"}]}"),
-		}}).Return(nil),
-		mock.EXPECT().Read(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).SetArg(3, []odooclient.ResPartner{{
+		}}, nil),
+		mock.EXPECT().FindResPartners(gomock.Any(), gomock.Any()).Return(&odooclient.ResPartners{{
 			Id:   odooclient.NewInt(123),
 			Name: odooclient.NewString("Test Company"),
-		}}).Return(nil),
+		}}, nil),
 	)
 
 	s, err := subject.Get(context.Background(), "be-456")
@@ -85,11 +85,10 @@ func TestGetNoParent(t *testing.T) {
 	defer ctrl.Finish()
 
 	gomock.InOrder(
-		mock.EXPECT().Read(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).SetArg(3, []odooclient.ResPartner{{
+		mock.EXPECT().FindResPartners(gomock.Any(), gomock.Any()).Return(&odooclient.ResPartners{{
 			Id:   odooclient.NewInt(456),
 			Name: odooclient.NewString("Accounting"),
-		}},
-		).Return(nil),
+		}}, nil),
 	)
 
 	_, err := subject.Get(context.Background(), "be-456")
@@ -101,12 +100,12 @@ func TestGet_ParentCantBeRetrieved(t *testing.T) {
 	defer ctrl.Finish()
 
 	gomock.InOrder(
-		mock.EXPECT().Read(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).SetArg(3, []odooclient.ResPartner{{
+		mock.EXPECT().FindResPartners(gomock.Any(), gomock.Any()).Return(&odooclient.ResPartners{{
 			Id:       odooclient.NewInt(456),
 			Name:     odooclient.NewString("Accounting"),
 			ParentId: odooclient.NewMany2One(123, ""),
-		}}).Return(nil),
-		mock.EXPECT().Read(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(errors.New("No such record")),
+		}}, nil),
+		mock.EXPECT().FindResPartners(gomock.Any(), gomock.Any()).Return(nil, errors.New("No such record")),
 	)
 
 	_, err := subject.Get(context.Background(), "be-456")
@@ -189,17 +188,17 @@ func TestCreate(t *testing.T) {
 		// Reset inflight flag
 		mock.EXPECT().Update(odooclient.ResPartnerModel, gomock.InAnyOrder([]int64{700, 702}), gomock.Any()),
 		// Fetch created company
-		mock.EXPECT().Read(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).SetArg(3, []odooclient.ResPartner{{
+		mock.EXPECT().FindResPartners(gomock.Any(), gomock.Any()).Return(&odooclient.ResPartners{{
 			Id:         odooclient.NewInt(702),
 			Name:       odooclient.NewString("Max Foobar"),
 			CreateDate: odooclient.NewTime(tn),
 			ParentId:   odooclient.NewMany2One(700, ""),
 			Email:      odooclient.NewString("[email protected], [email protected]"),
-		}}),
-		mock.EXPECT().Read(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).SetArg(3, []odooclient.ResPartner{{
+		}}, nil),
+		mock.EXPECT().FindResPartners(gomock.Any(), gomock.Any()).Return(&odooclient.ResPartners{{
 			Id:   odooclient.NewInt(700),
 			Name: odooclient.NewString("Test Company"),
-		}}),
+		}}, nil),
 	)
 
 	s := &billingv1.BillingEntity{
@@ -239,30 +238,30 @@ func TestUpdate(t *testing.T) {
 
 	gomock.InOrder(
 		// Fetch existing company
-		mock.EXPECT().Read(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).SetArg(3, []odooclient.ResPartner{{
+		mock.EXPECT().FindResPartners(gomock.Any(), gomock.Any()).Return(&odooclient.ResPartners{{
 			Id:       odooclient.NewInt(702),
 			ParentId: odooclient.NewMany2One(700, ""),
-		}}),
-		mock.EXPECT().Read(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).SetArg(3, []odooclient.ResPartner{{
+		}}, nil),
+		mock.EXPECT().FindResPartners(gomock.Any(), gomock.Any()).Return(&odooclient.ResPartners{{
 			Id:   odooclient.NewInt(700),
 			Name: odooclient.NewString("Test Company"),
-		}}),
+		}}, nil),
 		// Update company
 		mock.EXPECT().UpdateResPartner(gomock.Any()),
 		// Update accounting contact
 		mock.EXPECT().UpdateResPartner(gomock.Any()),
 		// Fetch created company
-		mock.EXPECT().Read(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).SetArg(3, []odooclient.ResPartner{{
+		mock.EXPECT().FindResPartners(gomock.Any(), gomock.Any()).Return(&odooclient.ResPartners{{
 			Id:                       odooclient.NewInt(702),
 			CreateDate:               odooclient.NewTime(tn),
 			ParentId:                 odooclient.NewMany2One(700, ""),
 			Email:                    odooclient.NewString("[email protected], [email protected]"),
 			VshnControlApiMetaStatus: odooclient.NewString("{\"conditions\":[{\"type\":\"ConditionFoo\",\"status\":\"False\",\"lastTransitionTime\":\"" + statusTime.Format(time.RFC3339) + "\",\"reason\":\"Whatever\",\"message\":\"Hello World\"}]}"),
-		}}),
-		mock.EXPECT().Read(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).SetArg(3, []odooclient.ResPartner{{
+		}}, nil),
+		mock.EXPECT().FindResPartners(gomock.Any(), gomock.Any()).Return(&odooclient.ResPartners{{
 			Id:   odooclient.NewInt(700),
 			Name: odooclient.NewString("Test Company"),
-		}}),
+		}}, nil),
 	)
 
 	s := &billingv1.BillingEntity{

apiserver/organization/billingentity_validate.go

@@ -3,6 +3,7 @@ package organization
 import (
 	"context"
 	"fmt"
+	"os"
 
 	"k8s.io/apiserver/pkg/authentication/user"
 	"k8s.io/apiserver/pkg/endpoints/request"
@@ -45,29 +46,38 @@ func (c impersonatorFromRestconf) Impersonate(u user.Info) (client.Client, error
 // billingEntityValidator validates that the billing entity exists and the requesting user has access to it.
 // it does so by impersonating the user and trying to get the billing entity.
 func (s *organizationStorage) billingEntityValidator(ctx context.Context, org, oldOrg *orgv1.Organization) error {
-	// check if changed
-	if oldOrg != nil && oldOrg.Spec.BillingEntityRef == org.Spec.BillingEntityRef {
-		return nil
-	}
-	// check if we allow empty billing entities
-	if org.Spec.BillingEntityRef == "" && s.allowEmptyBillingEntity {
-		return nil
-	}
+	validate := func() error {
+		// check if changed
+		if oldOrg != nil && oldOrg.Spec.BillingEntityRef == org.Spec.BillingEntityRef {
+			return nil
+		}
+		// check if we allow empty billing entities
+		if org.Spec.BillingEntityRef == "" && s.allowEmptyBillingEntity {
+			return nil
+		}
 
-	user, ok := request.UserFrom(ctx)
-	if !ok {
-		return fmt.Errorf("no user in context")
-	}
+		user, ok := request.UserFrom(ctx)
+		if !ok {
+			return fmt.Errorf("no user in context")
+		}
 
-	var be billingv1.BillingEntity
-	c, err := s.impersonator.Impersonate(user)
-	if err != nil {
-		return fmt.Errorf("failed to impersonate user: %w", err)
-	}
+		var be billingv1.BillingEntity
+		c, err := s.impersonator.Impersonate(user)
+		if err != nil {
+			return fmt.Errorf("failed to impersonate user: %w", err)
+		}
+
+		if err := c.Get(ctx, client.ObjectKey{Name: org.Spec.BillingEntityRef}, &be); err != nil {
+			return err
+		}
 
-	if err := c.Get(ctx, client.ObjectKey{Name: org.Spec.BillingEntityRef}, &be); err != nil {
-		return err
+		return nil
 	}
 
-	return nil
+	err := validate()
+	if err != nil && s.skipBillingEntityValidation {
+		fmt.Fprintf(os.Stderr, "Warning: billing entity validation for %q (ref: %q) would have failed: %v\n", org.Name, org.Spec.BillingEntityRef, err)
+		return nil
+	}
+	return err
 }