Skip to content

Commit

Permalink
Merge branch 'main' into update-patching
Browse files Browse the repository at this point in the history
  • Loading branch information
cam-garrison authored Sep 4, 2024
2 parents b89f879 + 5915fa2 commit 270196a
Show file tree
Hide file tree
Showing 16 changed files with 355 additions and 234 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/build.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,4 @@ jobs:
- uses: actions/setup-go@v5
with:
go-version-file: go.mod
- run: make
- run: make test
7 changes: 5 additions & 2 deletions controllers/authzctrl/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func New(cli client.Client, log logr.Logger,
hostExtractor: spi.UnifiedHostExtractor(
spi.NewPathExpressionExtractor(protectedResource.HostPaths),
spi.NewAnnotationHostExtractor(";", metadata.Keys(annotations.RoutingAddressesExternal(""), annotations.RoutingAddressesPublic(""))...)),
templateLoader: authorization.NewConfigMapTemplateLoader(cli, authorization.NewStaticTemplateLoader(config.Audiences)),
templateLoader: authorization.NewConfigMapTemplateLoader(cli, authorization.NewStaticTemplateLoader()),
}
}

Expand Down Expand Up @@ -120,8 +120,11 @@ func (r *Controller) SetupWithManager(mgr ctrl.Manager) error {
Complete(r)
}

func (r *Controller) Activate() {
var _ platformctrl.Activable[authorization.ProviderConfig] = &Controller{}

func (r *Controller) Activate(config authorization.ProviderConfig) {
r.active = true
r.config = config
}

func (r *Controller) Deactivate() {
Expand Down
7 changes: 6 additions & 1 deletion controllers/authzctrl/reconcile_authconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,12 @@ func (r *Controller) createAuthConfigTemplate(ctx context.Context, target *unstr
return authorinov1beta2.AuthConfig{}, fmt.Errorf("could not detect authtype: %w", err)
}

templ, err := r.templateLoader.Load(ctx, authType, types.NamespacedName{Namespace: target.GetNamespace(), Name: target.GetName()})
templateData := map[string]any{
"Namespace": target.GetNamespace(),
"Audiences": r.config.Audiences,
}

templ, err := r.templateLoader.Load(ctx, authType, types.NamespacedName{Namespace: target.GetNamespace(), Name: target.GetName()}, templateData)
if err != nil {
return authorinov1beta2.AuthConfig{}, fmt.Errorf("could not load template %s: %w", authType, err)
}
Expand Down
5 changes: 4 additions & 1 deletion controllers/routingctrl/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,11 @@ func (r *Controller) SetupWithManager(mgr ctrl.Manager) error {
Complete(r)
}

func (r *Controller) Activate() {
var _ platformctrl.Activable[routing.IngressConfig] = &Controller{}

func (r *Controller) Activate(config routing.IngressConfig) {
r.active = true
r.config = config
}

func (r *Controller) Deactivate() {
Expand Down
292 changes: 151 additions & 141 deletions controllers/routingctrl/controller_test.go

Large diffs are not rendered by default.

56 changes: 53 additions & 3 deletions controllers/routingctrl/fixtures_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/opendatahub-io/odh-platform/pkg/metadata/labels"
"github.com/opendatahub-io/odh-platform/test"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
Expand Down Expand Up @@ -46,12 +47,61 @@ func addRoutingRequirementsToSvc(ctx context.Context, exportedSvc *corev1.Servic
Expect(errExportSvc).ToNot(HaveOccurred())
}

func createComponentRequiringPlatformRouting(ctx context.Context, componentName, mode, appNs string) (*unstructured.Unstructured, error) {
// createComponentRequiringPlatformRouting creates a new component with the specified routing modes.
func createComponentRequiringPlatformRouting(ctx context.Context, componentName, appNs string, modes ...annotations.RoutingExportMode) (*unstructured.Unstructured, error) {
component, errCreate := test.CreateUnstructured(componentResource(componentName, appNs))
Expect(errCreate).ToNot(HaveOccurred())

// set component's "routing.opendatahub.io/export-mode" annotation to the specified mode.
metadata.ApplyMetaOptions(component, annotations.RoutingExportMode(mode))
for _, mode := range modes {
metadata.ApplyMetaOptions(component, mode)
}

return component, envTest.Client.Create(ctx, component)
}

type exportModeAction struct {
mode string
value string
}

var (
enablePublic = exportModeAction{mode: annotations.PublicMode().Key(), value: "true"}
disableExternal = exportModeAction{mode: annotations.ExternalMode().Key(), value: "false"}
removeExternal = exportModeAction{mode: annotations.ExternalMode().Key(), value: ""}
removePublic = exportModeAction{mode: annotations.PublicMode().Key(), value: ""}
enableNonSupportedMode = exportModeAction{
mode: annotations.RoutingExportModePrefix + "notsupported",
value: "true",
}
)

func (m exportModeAction) ApplyToMeta(obj metav1.Object) {
annos := obj.GetAnnotations()
if annos == nil {
annos = make(map[string]string)
}

key := m.mode

if m.value == "" {
delete(annos, key)
} else {
annos[key] = m.value
}

obj.SetAnnotations(annos)
}

func setExportModes(ctx context.Context, component *unstructured.Unstructured, actions ...exportModeAction) {
errGetComponent := envTest.Client.Get(ctx, client.ObjectKey{
Namespace: component.GetNamespace(),
Name: component.GetName(),
}, component)
Expect(errGetComponent).ToNot(HaveOccurred())

for _, action := range actions {
metadata.ApplyMetaOptions(component, action)
}

Expect(envTest.Client.Update(ctx, component)).To(Succeed())
}
86 changes: 47 additions & 39 deletions controllers/routingctrl/reconcile_resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func (r *Controller) createRoutingResources(ctx context.Context, target *unstruc

renderedSelectors, errLables := config.ResolveSelectors(r.component.ServiceSelector, target)
if errLables != nil {
return fmt.Errorf("could not render labels for ServiceSelector %v. Error %w", r.component.ServiceSelector, errLables)
return fmt.Errorf("could not render labels for ServiceSelector %v: %w", r.component.ServiceSelector, errLables)
}

exportedServices, errSvcGet := getExportedServices(ctx, r.Client, renderedSelectors, target)
Expand Down Expand Up @@ -68,45 +68,51 @@ func (r *Controller) createRoutingResources(ctx context.Context, target *unstruc
func (r *Controller) exportService(ctx context.Context, target *unstructured.Unstructured, exportedSvc *corev1.Service, domain string) error {
exportModes := r.extractExportModes(target)

templateData := routing.NewExposedServiceConfig(exportedSvc, r.config, domain)
externalHosts := []string{}
publicHosts := []string{}

// To establish ownership for watched component
ownershipLabels := append(labels.AsOwner(target), labels.AppManagedBy("odh-routing-controller"))

for _, exportMode := range exportModes {
resources, err := r.templateLoader.Load(templateData, exportMode)
if err != nil {
return fmt.Errorf("could not load templates for type %s: %w", exportMode, err)
}

ownershipLabels = append(ownershipLabels, labels.ExportType(exportMode))
if errApply := unstruct.Apply(ctx, r.Client, resources, ownershipLabels...); errApply != nil {
return fmt.Errorf("could not apply routing resources for type %s: %w", exportMode, errApply)
for _, exportedSvcPort := range exportedSvc.Spec.Ports {
templateData := routing.NewExposedServiceConfig(exportedSvc, exportedSvcPort, r.config, domain)

for _, exportMode := range exportModes {
resources, err := r.templateLoader.Load(templateData, exportMode)
if err != nil {
return fmt.Errorf("could not load templates for type %s: %w", exportMode, err)
}

ownershipLabels = append(ownershipLabels, labels.ExportType(exportMode))
if errApply := unstruct.Apply(ctx, r.Client, resources, ownershipLabels...); errApply != nil {
return fmt.Errorf("could not apply routing resources for type %s: %w", exportMode, errApply)
}

switch exportMode {
case routing.ExternalRoute:
externalHosts = append(externalHosts, templateData.ExternalHost())
case routing.PublicRoute:
publicHosts = append(publicHosts, templateData.PublicHosts()...)
}
}
}

return r.propagateHostsToWatchedCR(target, templateData)
return r.propagateHostsToWatchedCR(target, publicHosts, externalHosts)
}

func (r *Controller) propagateHostsToWatchedCR(target *unstructured.Unstructured, data *routing.ExposedServiceConfig) error {
exportModes := r.extractExportModes(target)

func (r *Controller) propagateHostsToWatchedCR(target *unstructured.Unstructured, publicHosts, externalHosts []string) error {
// Remove all existing routing addresses
metaOptions := []metadata.Option{
annotations.Remove(annotations.RoutingAddressesExternal("")),
annotations.Remove(annotations.RoutingAddressesPublic("")),
}

// TODO(mvp): put the logic of creating host names into a single place
for _, exportMode := range exportModes {
switch exportMode {
case routing.ExternalRoute:
externalAddress := annotations.RoutingAddressesExternal(fmt.Sprintf("%s-%s.%s", data.ServiceName, data.ServiceNamespace, data.Domain))
metaOptions = append(metaOptions, externalAddress)
case routing.PublicRoute:
publicAddresses := annotations.RoutingAddressesPublic(fmt.Sprintf("%[1]s.%[2]s;%[1]s.%[2]s.svc;%[1]s.%[2]s.svc.cluster.local", data.PublicServiceName, data.GatewayNamespace))
metaOptions = append(metaOptions, publicAddresses)
}
if len(publicHosts) > 0 {
metaOptions = append(metaOptions, annotations.RoutingAddressesPublic(strings.Join(publicHosts, ";")))
}

if len(externalHosts) > 0 {
metaOptions = append(metaOptions, annotations.RoutingAddressesExternal(strings.Join(externalHosts, ";")))
}

metadata.ApplyMetaOptions(target, metaOptions...)
Expand All @@ -124,24 +130,26 @@ func (r *Controller) ensureResourceHasFinalizer(ctx context.Context, target *uns
return nil
}

// extractExportModes retrieves the enabled export modes from the target's annotations.
func (r *Controller) extractExportModes(target *unstructured.Unstructured) []routing.RouteType {
exportModes, exportModeFound := target.GetAnnotations()[annotations.RoutingExportMode("").Key()]
if !exportModeFound {
targetAnnotations := target.GetAnnotations()
if targetAnnotations == nil {
return nil
}

exportModesSplit := strings.Split(exportModes, ";")
validRouteTypes := make([]routing.RouteType, 0, len(exportModesSplit))

for _, exportMode := range exportModesSplit {
routeType := routing.RouteType(strings.TrimSpace(exportMode))
if routing.IsValidRouteType(routeType) {
validRouteTypes = append(validRouteTypes, routeType)
} else {
r.log.Info("Invalid route type found",
"invalidRouteType", routeType,
"resourceName", target.GetName(),
"resourceNamespace", target.GetNamespace())
validRouteTypes := make([]routing.RouteType, 0)

for key, value := range targetAnnotations {
if value == "true" {
routeType, valid := routing.IsValidRouteType(key)
if valid {
validRouteTypes = append(validRouteTypes, routeType)
} else {
r.log.Info("Invalid route type found",
"invalidRouteType", routeType,
"resourceName", target.GetName(),
"resourceNamespace", target.GetNamespace())
}
}
}

Expand Down
4 changes: 2 additions & 2 deletions controllers/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ import (
ctrl "sigs.k8s.io/controller-runtime"
)

type Activable interface {
Activate()
type Activable[T any] interface {
Activate(config T)
Deactivate()
}

Expand Down
18 changes: 6 additions & 12 deletions pkg/authorization/authconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,23 +36,17 @@ var authConfigTemplateAnonymous []byte
var authConfigTemplateUserDefined []byte

type staticTemplateLoader struct {
audience []string
}

var _ AuthConfigTemplateLoader = (*staticTemplateLoader)(nil)

func NewStaticTemplateLoader(audience []string) *staticTemplateLoader {
return &staticTemplateLoader{audience: audience}
func NewStaticTemplateLoader() *staticTemplateLoader {
return &staticTemplateLoader{}
}

func (s *staticTemplateLoader) Load(_ context.Context, authType AuthType, key types.NamespacedName) (authorinov1beta2.AuthConfig, error) {
func (s *staticTemplateLoader) Load(_ context.Context, authType AuthType, key types.NamespacedName, templateData map[string]any) (authorinov1beta2.AuthConfig, error) {
authConfig := authorinov1beta2.AuthConfig{}

templateData := map[string]interface{}{
"Namespace": key.Namespace,
"Audiences": s.audience,
}

templateContent := authConfigTemplateAnonymous
if authType == UserDefined {
templateContent = authConfigTemplateUserDefined
Expand All @@ -71,7 +65,7 @@ func (s *staticTemplateLoader) Load(_ context.Context, authType AuthType, key ty
return authConfig, nil
}

func (s *staticTemplateLoader) resolveTemplate(tmpl []byte, data map[string]interface{}) ([]byte, error) {
func (s *staticTemplateLoader) resolveTemplate(tmpl []byte, data map[string]any) ([]byte, error) {
engine, err := template.New("authconfig").Parse(string(tmpl))
if err != nil {
return []byte{}, fmt.Errorf("could not create template engine: %w", err)
Expand Down Expand Up @@ -103,9 +97,9 @@ func NewConfigMapTemplateLoader(cli client.Client, fallback AuthConfigTemplateLo

// TODO: check "authconfig-template" CM in key.Namespace to see if there is a "spec" to use, construct a AuthConfig object
// https://issues.redhat.com/browse/RHOAIENG-847
func (c *configMapTemplateLoader) Load(ctx context.Context, authType AuthType, key types.NamespacedName) (authorinov1beta2.AuthConfig, error) {
func (c *configMapTemplateLoader) Load(ctx context.Context, authType AuthType, key types.NamespacedName, templateData map[string]any) (authorinov1beta2.AuthConfig, error) {
// else
ac, err := c.fallback.Load(ctx, authType, key)
ac, err := c.fallback.Load(ctx, authType, key, templateData)
if err != nil {
return authorinov1beta2.AuthConfig{}, fmt.Errorf("could not load from fallback: %w", err)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/authorization/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,5 +37,5 @@ type AuthTypeDetector interface {
// - Namespace / Resource name
// - Loader source
type AuthConfigTemplateLoader interface {
Load(ctx context.Context, authType AuthType, key types.NamespacedName) (v1beta2.AuthConfig, error)
Load(ctx context.Context, authType AuthType, key types.NamespacedName, templateData map[string]any) (v1beta2.AuthConfig, error)
}
27 changes: 21 additions & 6 deletions pkg/metadata/annotations/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

const (
RoutingExportModePrefix = "routing.opendatahub.io/export-mode-"
)

type Annotation interface {
metadata.Option
metadata.KeyValue
Expand Down Expand Up @@ -44,21 +48,32 @@ func (a AuthorizationGroup) Value() string {
return string(a)
}

// RoutingExportMode defines the export mode for the routing capability. It can be
// either "public" or "external" or both, delimited by ";".
// It is intended to be defined on the component's Custom Resource.
type RoutingExportMode string
// RoutingExportMode defines an individual export mode for the routing capability.
// Each mode (currently: "public" or "external") is represented by a separate annotation.
// The annotation key is formed by prefixing the mode with "routing.opendatahub.io/export-mode-", value is boolean.
type RoutingExportMode struct {
mode string
value string
}

func ExternalMode() RoutingExportMode {
return RoutingExportMode{mode: "external", value: "true"}
}

func PublicMode() RoutingExportMode {
return RoutingExportMode{mode: "public", value: "true"}
}

func (r RoutingExportMode) ApplyToMeta(obj metav1.Object) {
addAnnotation(r, obj)
}

func (r RoutingExportMode) Key() string {
return "routing.opendatahub.io/export-mode"
return RoutingExportModePrefix + r.mode
}

func (r RoutingExportMode) Value() string {
return string(r)
return r.value
}

// RoutingAddressesPublic exposes the public addresses set by Platform's routing capability.
Expand Down
Loading

0 comments on commit 270196a

Please sign in to comment.