Skip to content

Commit

Permalink
Add some generic type hook wrappers to first decode the data
Browse files Browse the repository at this point in the history 
There seems to be a pattern for Validation, Mutation and Write Authorization hooks where they first need to decode the Any data before doing the domain specific work.

This PR introduces 3 new functions to generate wrappers around the other hooks to pre-decode the data into a DecodedResource and pass that in instead of the original pbresource.Resource.

This PR also updates the various catalog data types to use the new hook generators.
  • Loading branch information
@mkeeler
mkeeler committed Aug 22, 2023
1 parent c4b3234 commit f3b195a
Show file tree
Hide file tree
Showing 13 changed files with 341 additions and 127 deletions.
16 changes: 7 additions & 9 deletions internal/catalog/internal/types/dns_policy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,12 @@ var (
}

DNSPolicyType = DNSPolicyV1Alpha1Type

ValidateDNSPolicy = resource.DecodeAndValidate(validateDNSPolicy)
)

type DecodedDNSPolicy = resource.DecodedResource[*pbcatalog.DNSPolicy]

func RegisterDNSPolicy(r resource.Registry) {
r.Register(resource.Registration{
Type: DNSPolicyV1Alpha1Type,
Expand All @@ -34,25 +38,19 @@ func RegisterDNSPolicy(r resource.Registry) {
})
}

func ValidateDNSPolicy(res *pbresource.Resource) error {
var policy pbcatalog.DNSPolicy

if err := res.Data.UnmarshalTo(&policy); err != nil {
return resource.NewErrDataParse(&policy, err)
}

func validateDNSPolicy(dec *DecodedDNSPolicy) error {
var err error
// Ensure that this resource isn't useless and is attempting to
// select at least one workload.
if selErr := validateSelector(policy.Workloads, false); selErr != nil {
if selErr := validateSelector(dec.Data.Workloads, false); selErr != nil {
err = multierror.Append(err, resource.ErrInvalidField{
Name: "workloads",
Wrapped: selErr,
})
}

// Validate the weights
if weightErr := validateDNSPolicyWeights(policy.Weights); weightErr != nil {
if weightErr := validateDNSPolicyWeights(dec.Data.Weights); weightErr != nil {
err = multierror.Append(err, resource.ErrInvalidField{
Name: "weights",
Wrapped: weightErr,
Expand Down
47 changes: 18 additions & 29 deletions internal/catalog/internal/types/failover_policy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,13 @@ var (
}

FailoverPolicyType = FailoverPolicyV1Alpha1Type

ValidateFailoverPolicy = resource.DecodeAndValidate[*pbcatalog.FailoverPolicy](validateFailoverPolicy)
MutateFailoverPolicy = resource.DecodeAndMutate[*pbcatalog.FailoverPolicy](mutateFailoverPolicy)
)

type DecodedFailoverPolicy = resource.DecodedResource[*pbcatalog.FailoverPolicy]

func RegisterFailoverPolicy(r resource.Registry) {
r.Register(resource.Registration{
Type: FailoverPolicyV1Alpha1Type,
Expand All @@ -38,66 +43,50 @@ func RegisterFailoverPolicy(r resource.Registry) {
})
}

func MutateFailoverPolicy(res *pbresource.Resource) error {
var failover pbcatalog.FailoverPolicy

if err := res.Data.UnmarshalTo(&failover); err != nil {
return resource.NewErrDataParse(&failover, err)
}

func mutateFailoverPolicy(dec *DecodedFailoverPolicy) (bool, error) {
changed := false

// Handle eliding empty configs.
if failover.Config != nil && failover.Config.IsEmpty() {
failover.Config = nil
if dec.Data.Config != nil && dec.Data.Config.IsEmpty() {
dec.Data.Config = nil
changed = true
}
for port, pc := range failover.PortConfigs {
for port, pc := range dec.Data.PortConfigs {
if pc.IsEmpty() {
delete(failover.PortConfigs, port)
delete(dec.Data.PortConfigs, port)
changed = true
}
}
if len(failover.PortConfigs) == 0 {
failover.PortConfigs = nil
if len(dec.Data.PortConfigs) == 0 {
dec.Data.PortConfigs = nil
changed = true
}

// TODO(rb): normalize dest ref tenancies

if !changed {
return nil
}

return res.Data.MarshalFrom(&failover)
return changed, nil
}

func ValidateFailoverPolicy(res *pbresource.Resource) error {
var failover pbcatalog.FailoverPolicy

if err := res.Data.UnmarshalTo(&failover); err != nil {
return resource.NewErrDataParse(&failover, err)
}

func validateFailoverPolicy(dec *DecodedFailoverPolicy) error {
var merr error

if failover.Config == nil && len(failover.PortConfigs) == 0 {
if dec.Data.Config == nil && len(dec.Data.PortConfigs) == 0 {
merr = multierror.Append(merr, resource.ErrInvalidField{
Name: "config",
Wrapped: fmt.Errorf("at least one of config or port_configs must be set"),
})
}

if failover.Config != nil {
for _, err := range validateFailoverConfig(failover.Config, false) {
if dec.Data.Config != nil {
for _, err := range validateFailoverConfig(dec.Data.Config, false) {
merr = multierror.Append(merr, resource.ErrInvalidField{
Name: "config",
Wrapped: err,
})
}
}

for portName, pc := range failover.PortConfigs {
for portName, pc := range dec.Data.PortConfigs {
if portNameErr := validatePortName(portName); portNameErr != nil {
merr = multierror.Append(merr, resource.ErrInvalidMapKey{
Map: "port_configs",
Expand Down
16 changes: 7 additions & 9 deletions internal/catalog/internal/types/health_checks.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,12 @@ var (
}

HealthChecksType = HealthChecksV1Alpha1Type

ValidateHealthChecks = resource.DecodeAndValidate[*pbcatalog.HealthChecks](validateHealthChecks)
)

type DecodedHealthChecks = resource.DecodedResource[*pbcatalog.HealthChecks]

func RegisterHealthChecks(r resource.Registry) {
r.Register(resource.Registration{
Type: HealthChecksV1Alpha1Type,
Expand All @@ -32,25 +36,19 @@ func RegisterHealthChecks(r resource.Registry) {
})
}

func ValidateHealthChecks(res *pbresource.Resource) error {
var checks pbcatalog.HealthChecks

if err := res.Data.UnmarshalTo(&checks); err != nil {
return resource.NewErrDataParse(&checks, err)
}

func validateHealthChecks(dec *DecodedHealthChecks) error {
var err error

// Validate the workload selector
if selErr := validateSelector(checks.Workloads, false); selErr != nil {
if selErr := validateSelector(dec.Data.Workloads, false); selErr != nil {
err = multierror.Append(err, resource.ErrInvalidField{
Name: "workloads",
Wrapped: selErr,
})
}

// Validate each check
for idx, check := range checks.HealthChecks {
for idx, check := range dec.Data.HealthChecks {
if checkErr := validateCheck(check); checkErr != nil {
err = multierror.Append(err, resource.ErrInvalidListElement{
Name: "checks",
Expand Down
22 changes: 10 additions & 12 deletions internal/catalog/internal/types/health_status.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,12 @@ var (
}

HealthStatusType = HealthStatusV1Alpha1Type

ValidateHealthStatus = resource.DecodeAndValidate[*pbcatalog.HealthStatus](validateHealthStatus)
)

type DecodedHealthStatus = resource.DecodedResource[*pbcatalog.HealthStatus]

func RegisterHealthStatus(r resource.Registry) {
r.Register(resource.Registration{
Type: HealthStatusV1Alpha1Type,
Expand All @@ -32,26 +36,20 @@ func RegisterHealthStatus(r resource.Registry) {
})
}

func ValidateHealthStatus(res *pbresource.Resource) error {
var hs pbcatalog.HealthStatus

if err := res.Data.UnmarshalTo(&hs); err != nil {
return resource.NewErrDataParse(&hs, err)
}

func validateHealthStatus(dec *DecodedHealthStatus) error {
var err error

// Should we allow empty types? I think for now it will be safest to require
// the type field is set and we can relax this restriction in the future
// if we deem it desirable.
if hs.Type == "" {
if dec.Data.Type == "" {
err = multierror.Append(err, resource.ErrInvalidField{
Name: "type",
Wrapped: resource.ErrMissing,
})
}

switch hs.Status {
switch dec.Data.Status {
case pbcatalog.Health_HEALTH_PASSING,
pbcatalog.Health_HEALTH_WARNING,
pbcatalog.Health_HEALTH_CRITICAL,
Expand All @@ -67,13 +65,13 @@ func ValidateHealthStatus(res *pbresource.Resource) error {
// owner is currently the resource that this HealthStatus applies to. If we
// change this to be a parent reference within the HealthStatus.Data then
// we could allow for other owners.
if res.Owner == nil {
if dec.Resource.Owner == nil {
err = multierror.Append(err, resource.ErrInvalidField{
Name: "owner",
Wrapped: resource.ErrMissing,
})
} else if !resource.EqualType(res.Owner.Type, WorkloadType) && !resource.EqualType(res.Owner.Type, NodeType) {
err = multierror.Append(err, resource.ErrOwnerTypeInvalid{ResourceType: res.Id.Type, OwnerType: res.Owner.Type})
} else if !resource.EqualType(dec.Resource.Owner.Type, WorkloadType) && !resource.EqualType(dec.Resource.Owner.Type, NodeType) {
err = multierror.Append(err, resource.ErrOwnerTypeInvalid{ResourceType: dec.Resource.Id.Type, OwnerType: dec.Resource.Owner.Type})
}

return err
Expand Down
16 changes: 7 additions & 9 deletions internal/catalog/internal/types/node.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,12 @@ var (
}

NodeType = NodeV1Alpha1Type

ValidateNode = resource.DecodeAndValidate[*pbcatalog.Node](validateNode)
)

type DecodedNode = resource.DecodedResource[*pbcatalog.Node]

func RegisterNode(r resource.Registry) {
r.Register(resource.Registration{
Type: NodeV1Alpha1Type,
Expand All @@ -32,24 +36,18 @@ func RegisterNode(r resource.Registry) {
})
}

func ValidateNode(res *pbresource.Resource) error {
var node pbcatalog.Node

if err := res.Data.UnmarshalTo(&node); err != nil {
return resource.NewErrDataParse(&node, err)
}

func validateNode(dec *DecodedNode) error {
var err error
// Validate that the node has at least 1 address
if len(node.Addresses) < 1 {
if len(dec.Data.Addresses) < 1 {
err = multierror.Append(err, resource.ErrInvalidField{
Name: "addresses",
Wrapped: resource.ErrEmpty,
})
}

// Validate each node address
for idx, addr := range node.Addresses {
for idx, addr := range dec.Data.Addresses {
if addrErr := validateNodeAddress(addr); addrErr != nil {
err = multierror.Append(err, resource.ErrInvalidListElement{
Name: "addresses",
Expand Down
18 changes: 8 additions & 10 deletions internal/catalog/internal/types/service.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,12 @@ var (
}

ServiceType = ServiceV1Alpha1Type

ValidateService = resource.DecodeAndValidate[*pbcatalog.Service](validateService)
)

type DecodedService = resource.DecodedResource[*pbcatalog.Service]

func RegisterService(r resource.Registry) {
r.Register(resource.Registration{
Type: ServiceV1Alpha1Type,
Expand All @@ -34,21 +38,15 @@ func RegisterService(r resource.Registry) {
})
}

func ValidateService(res *pbresource.Resource) error {
var service pbcatalog.Service

if err := res.Data.UnmarshalTo(&service); err != nil {
return resource.NewErrDataParse(&service, err)
}

func validateService(dec *DecodedService) error {
var err error

// Validate the workload selector. We are allowing selectors with no
// selection criteria as it will allow for users to manually manage
// ServiceEndpoints objects for this service such as when desiring to
// configure endpoint information for external services that are not
// registered as workloads
if selErr := validateSelector(service.Workloads, true); selErr != nil {
if selErr := validateSelector(dec.Data.Workloads, true); selErr != nil {
err = multierror.Append(err, resource.ErrInvalidField{
Name: "workloads",
Wrapped: selErr,
Expand All @@ -58,7 +56,7 @@ func ValidateService(res *pbresource.Resource) error {
usedVirtualPorts := make(map[uint32]int)

// Validate each port
for idx, port := range service.Ports {
for idx, port := range dec.Data.Ports {
if usedIdx, found := usedVirtualPorts[port.VirtualPort]; found {
err = multierror.Append(err, resource.ErrInvalidListElement{
Name: "ports",
Expand Down Expand Up @@ -105,7 +103,7 @@ func ValidateService(res *pbresource.Resource) error {
}

// Validate that the Virtual IPs are all IP addresses
for idx, vip := range service.VirtualIps {
for idx, vip := range dec.Data.VirtualIps {
if vipErr := validateIPAddress(vip); vipErr != nil {
err = multierror.Append(err, resource.ErrInvalidListElement{
Name: "virtual_ips",
Expand Down
32 changes: 15 additions & 17 deletions internal/catalog/internal/types/service_endpoints.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,12 @@ var (
}

ServiceEndpointsType = ServiceEndpointsV1Alpha1Type

ValidateServiceEndpoints = resource.DecodeAndValidate[*pbcatalog.ServiceEndpoints](validateServiceEndpoints)
)

type DecodedServiceEndpoints = resource.DecodedResource[*pbcatalog.ServiceEndpoints]

func RegisterServiceEndpoints(r resource.Registry) {
r.Register(resource.Registration{
Type: ServiceEndpointsV1Alpha1Type,
Expand All @@ -47,41 +51,35 @@ func MutateServiceEndpoints(res *pbresource.Resource) error {
return nil
}

func ValidateServiceEndpoints(res *pbresource.Resource) error {
var svcEndpoints pbcatalog.ServiceEndpoints

if err := res.Data.UnmarshalTo(&svcEndpoints); err != nil {
return resource.NewErrDataParse(&svcEndpoints, err)
}

func validateServiceEndpoints(dec *DecodedServiceEndpoints) error {
var err error
if !resource.EqualType(res.Owner.Type, ServiceV1Alpha1Type) {
if !resource.EqualType(dec.Owner.Type, ServiceV1Alpha1Type) {
err = multierror.Append(err, resource.ErrOwnerTypeInvalid{
ResourceType: ServiceEndpointsV1Alpha1Type,
OwnerType: res.Owner.Type,
OwnerType: dec.Owner.Type,
})
}

if !resource.EqualTenancy(res.Owner.Tenancy, res.Id.Tenancy) {
if !resource.EqualTenancy(dec.Owner.Tenancy, dec.Id.Tenancy) {
err = multierror.Append(err, resource.ErrOwnerTenantInvalid{
ResourceType: ServiceEndpointsV1Alpha1Type,
ResourceTenancy: res.Id.Tenancy,
OwnerTenancy: res.Owner.Tenancy,
ResourceTenancy: dec.Id.Tenancy,
OwnerTenancy: dec.Owner.Tenancy,
})
}

if res.Owner.Name != res.Id.Name {
if dec.Owner.Name != dec.Id.Name {
err = multierror.Append(err, resource.ErrInvalidField{
Name: "name",
Wrapped: errInvalidEndpointsOwnerName{
Name: res.Id.Name,
OwnerName: res.Owner.Name,
Name: dec.Id.Name,
OwnerName: dec.Owner.Name,
},
})
}

for idx, endpoint := range svcEndpoints.Endpoints {
if endpointErr := validateEndpoint(endpoint, res); endpointErr != nil {
for idx, endpoint := range dec.Data.Endpoints {
if endpointErr := validateEndpoint(endpoint, dec.Resource); endpointErr != nil {
err = multierror.Append(err, resource.ErrInvalidListElement{
Name: "endpoints",
Index: idx,
Expand Down
Loading

0 comments on commit f3b195a

Please sign in to comment.