Skip to content

Commit

Permalink
chore(oauth): remove modifications to OpenShift api-server
Browse files Browse the repository at this point in the history
Signed-off-by: Thuan Vo <thvo@redhat.com>
  • Loading branch information
Thuan Vo committed Jul 13, 2023
1 parent 1a02ba8 commit 01cac41
Show file tree
Hide file tree
Showing 3 changed files with 2 additions and 112 deletions.
90 changes: 2 additions & 88 deletions internal/controllers/openshift.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,40 +39,29 @@ package controllers
import (
"context"
"fmt"
"regexp"

"github.com/cryostatio/cryostat-operator/internal/controllers/common"
"github.com/cryostatio/cryostat-operator/internal/controllers/model"
"github.com/go-logr/logr"
configv1 "github.com/openshift/api/config/v1"
consolev1 "github.com/openshift/api/console/v1"
kerrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
)

func (r *Reconciler) reconcileOpenShift(ctx context.Context, cr *model.CryostatInstance) error {
if !r.IsOpenShift {
return nil
}
err := r.reconcileConsoleLink(ctx, cr)
if err != nil {
return err
}
return r.addCorsAllowedOriginIfNotPresent(ctx, cr)
return r.reconcileConsoleLink(ctx, cr)
}

func (r *Reconciler) finalizeOpenShift(ctx context.Context, cr *model.CryostatInstance) error {
if !r.IsOpenShift {
return nil
}
reqLogger := r.Log.WithValues("Request.Namespace", cr.InstallNamespace, "Request.Name", cr.Name)
err := r.deleteConsoleLink(ctx, newConsoleLink(cr), reqLogger)
if err != nil {
return err
}
return r.deleteCorsAllowedOrigins(ctx, cr)
return r.deleteConsoleLink(ctx, newConsoleLink(cr), reqLogger)
}

func newConsoleLink(cr *model.CryostatInstance) *consolev1.ConsoleLink {
Expand Down Expand Up @@ -129,78 +118,3 @@ func (r *Reconciler) deleteConsoleLink(ctx context.Context, link *consolev1.Cons
logger.Info("deleted ConsoleLink", "name", link.Name)
return nil
}

func (r *Reconciler) addCorsAllowedOriginIfNotPresent(ctx context.Context, cr *model.CryostatInstance) error {
reqLogger := r.Log.WithValues("Request.Namespace", cr.InstallNamespace, "Request.Name", cr.Name)

allowedOrigin := cr.Status.ApplicationURL
if len(allowedOrigin) == 0 {
return nil
}

apiServer := &configv1.APIServer{}
err := r.Client.Get(ctx, types.NamespacedName{Name: apiServerName}, apiServer)
if err != nil {
reqLogger.Error(err, "Failed to get APIServer config")
return err
}

allowedOriginAsRegex := regexp.QuoteMeta(allowedOrigin)

for _, origin := range apiServer.Spec.AdditionalCORSAllowedOrigins {
if origin == allowedOriginAsRegex {
return nil
}
}

apiServer.Spec.AdditionalCORSAllowedOrigins = append(
apiServer.Spec.AdditionalCORSAllowedOrigins,
allowedOriginAsRegex,
)

err = r.Client.Update(ctx, apiServer)
if err != nil {
reqLogger.Error(err, "Failed to update APIServer CORS allowed origins")
return err
}

reqLogger.Info("Added to APIServer CORS allowed origins")
return nil
}

func (r *Reconciler) deleteCorsAllowedOrigins(ctx context.Context, cr *model.CryostatInstance) error {
reqLogger := r.Log.WithValues("Request.Namespace", cr.InstallNamespace, "Request.Name", cr.Name)

allowedOrigin := cr.Status.ApplicationURL
if len(allowedOrigin) == 0 {
reqLogger.Info("No Route to remove from APIServer config")
return nil
}

apiServer := &configv1.APIServer{}
err := r.Client.Get(ctx, types.NamespacedName{Name: apiServerName}, apiServer)
if err != nil {
reqLogger.Error(err, "Failed to get APIServer config")
return err
}

allowedOriginAsRegex := regexp.QuoteMeta(allowedOrigin)

for i, origin := range apiServer.Spec.AdditionalCORSAllowedOrigins {
if origin == allowedOriginAsRegex {
apiServer.Spec.AdditionalCORSAllowedOrigins = append(
apiServer.Spec.AdditionalCORSAllowedOrigins[:i],
apiServer.Spec.AdditionalCORSAllowedOrigins[i+1:]...)
break
}
}

err = r.Client.Update(ctx, apiServer)
if err != nil {
reqLogger.Error(err, "Failed to remove Cryostat origin from APIServer CORS allowed origins")
return err
}

reqLogger.Info("Removed from APIServer CORS allowed origins")
return nil
}
21 changes: 0 additions & 21 deletions internal/controllers/reconciler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ import (
certv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
configv1 "github.com/openshift/api/config/v1"
consolev1 "github.com/openshift/api/console/v1"
openshiftv1 "github.com/openshift/api/route/v1"
appsv1 "k8s.io/api/apps/v1"
Expand Down Expand Up @@ -1282,9 +1281,6 @@ func (c *controllerTest) commonTests() {
Expect(link.Spec.NamespaceDashboard).To(Equal(expectedLink.Spec.NamespaceDashboard))
})
})
It("should add application url to APIServer AdditionalCORSAllowedOrigins", func() {
t.expectAPIServer()
})
It("should add the finalizer", func() {
t.expectCryostatFinalizerPresent()
})
Expand Down Expand Up @@ -1315,13 +1311,6 @@ func (c *controllerTest) commonTests() {
err := t.Client.Get(context.Background(), types.NamespacedName{Name: expectedLink.Name}, link)
Expect(kerrors.IsNotFound(err)).To(BeTrue())
})
It("should remove the application url from APIServer AdditionalCORSAllowedOrigins", func() {
apiServer := &configv1.APIServer{}
err := t.Client.Get(context.Background(), types.NamespacedName{Name: "cluster"}, apiServer)
Expect(err).ToNot(HaveOccurred())
Expect(apiServer.Spec.AdditionalCORSAllowedOrigins).ToNot(ContainElement("https://cryostat\\.example\\.com"))
Expect(apiServer.Spec.AdditionalCORSAllowedOrigins).To(ContainElement("https://an-existing-user-specified\\.allowed\\.origin\\.com"))
})
It("should delete Cryostat", func() {
t.expectNoCryostat()
})
Expand Down Expand Up @@ -1811,9 +1800,6 @@ func (c *controllerTest) commonTests() {
It("should not delete exisiting ConsoleLink", func() {
otherInput.expectConsoleLink()
})
It("should not remove CORS entry from APIServer", func() {
otherInput.expectAPIServer()
})
})
})
})
Expand Down Expand Up @@ -2966,13 +2952,6 @@ func (t *cryostatTestInput) expectConsoleLink() {
Expect(link.Spec).To(Equal(expectedLink.Spec))
}

func (t *cryostatTestInput) expectAPIServer() {
apiServer := &configv1.APIServer{}
err := t.Client.Get(context.Background(), types.NamespacedName{Name: "cluster"}, apiServer)
Expect(err).ToNot(HaveOccurred())
Expect(apiServer.Spec.AdditionalCORSAllowedOrigins).To(ContainElement(fmt.Sprintf("https://%s\\.example\\.com", t.Name)))
}

func (t *cryostatTestInput) expectResourcesUnaffected() {
for _, check := range resourceChecks() {
check.expectFunc(t)
Expand Down
3 changes: 0 additions & 3 deletions internal/test/resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -2630,9 +2630,6 @@ func (r *TestResources) NewApiServer() *configv1.APIServer {
ObjectMeta: metav1.ObjectMeta{
Name: "cluster",
},
Spec: configv1.APIServerSpec{
AdditionalCORSAllowedOrigins: []string{"https://an-existing-user-specified\\.allowed\\.origin\\.com"},
},
}
}

Expand Down

0 comments on commit 01cac41

Please sign in to comment.