Skip to content

Commit

Permalink
fix: make default knative secret local for serverless (opendatahub-io…
Browse files Browse the repository at this point in the history
…#1067)

PR opendatahub-io#1022 promoted knative default secret name as the default for entire platform. This can be confusing, therefore this change makes it local to kserve again.
  • Loading branch information
bartoszmajsak authored and openshift-merge-bot[bot] committed Jun 26, 2024
1 parent 9d212cf commit 114395b
Show file tree
Hide file tree
Showing 5 changed files with 11 additions and 15 deletions.
12 changes: 3 additions & 9 deletions pkg/cluster/cert.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"
)

const DefaultCertificateSecretName = "knative-serving-cert"

func CreateSelfSignedCertificate(ctx context.Context, c client.Client, secretName, domain, namespace string, metaOptions ...MetaOptions) error {
certSecret, err := GenerateSelfSignedCertificateAsSecret(secretName, domain, namespace)
if err != nil {
Expand Down Expand Up @@ -125,8 +123,8 @@ func generateCertificate(addr string) ([]byte, []byte, error) {
return certBuffer.Bytes(), keyBuffer.Bytes(), nil
}

// GetDefaultIngressCertificate copies ingress cert secrets from openshift-ingress ns to given namespace.
func GetDefaultIngressCertificate(ctx context.Context, c client.Client, knativeSecret, namespace string) error {
// PropagateDefaultIngressCertificate copies ingress cert secrets from openshift-ingress ns to given namespace.
func PropagateDefaultIngressCertificate(ctx context.Context, c client.Client, secretName, namespace string) error {
// Add IngressController to scheme
runtime.Must(operatorv1.Install(c.Scheme()))
defaultIngressCtrl, err := FindAvailableIngressController(ctx, c)
Expand All @@ -141,7 +139,7 @@ func GetDefaultIngressCertificate(ctx context.Context, c client.Client, knativeS
return err
}

return copySecretToNamespace(ctx, c, defaultIngressSecret, knativeSecret, namespace)
return copySecretToNamespace(ctx, c, defaultIngressSecret, secretName, namespace)
}

func FindAvailableIngressController(ctx context.Context, c client.Client) (*operatorv1.IngressController, error) {
Expand Down Expand Up @@ -171,10 +169,6 @@ func GetSecret(ctx context.Context, c client.Client, namespace, name string) (*v
}

func copySecretToNamespace(ctx context.Context, c client.Client, secret *v1.Secret, newSecretName, namespace string) error {
// Get default name if newSecretName is empty
if newSecretName == "" {
newSecretName = DefaultCertificateSecretName
}
newSecret := &v1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: newSecretName,
Expand Down
4 changes: 3 additions & 1 deletion pkg/feature/serverless/loaders.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,12 @@ import (
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/feature"
)

const DefaultCertificateSecretName = "knative-serving-cert"

func ServingDefaultValues(f *feature.Feature) error {
certificateSecretName := strings.TrimSpace(f.Spec.Serving.IngressGateway.Certificate.SecretName)
if len(certificateSecretName) == 0 {
certificateSecretName = cluster.DefaultCertificateSecretName
certificateSecretName = DefaultCertificateSecretName
}

f.Spec.KnativeCertificateSecret = certificateSecretName
Expand Down
2 changes: 1 addition & 1 deletion pkg/feature/serverless/resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,6 @@ func ServingCertificateResource(f *feature.Feature) error {
case infrav1.Provided:
return nil
default:
return cluster.GetDefaultIngressCertificate(context.TODO(), f.Client, f.Spec.KnativeCertificateSecret, f.Spec.ControlPlane.Namespace)
return cluster.PropagateDefaultIngressCertificate(context.TODO(), f.Client, f.Spec.KnativeCertificateSecret, f.Spec.ControlPlane.Namespace)
}
}
3 changes: 2 additions & 1 deletion tests/e2e/dsc_creation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
infrav1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/infrastructure/v1"
"github.com/opendatahub-io/opendatahub-operator/v2/components"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/feature/serverless"
)

const (
Expand Down Expand Up @@ -393,7 +394,7 @@ func (tc *testContext) testDefaultCertsAvailable() error {
// Verify secret from Control Plane namespace matches the default cert secret
defaultSecretName := tc.testDsc.Spec.Components.Kserve.Serving.IngressGateway.Certificate.SecretName
if defaultSecretName == "" {
defaultSecretName = cluster.DefaultCertificateSecretName
defaultSecretName = serverless.DefaultCertificateSecretName
}
ctrlPlaneSecret, err := cluster.GetSecret(tc.ctx, tc.customClient, tc.testDSCI.Spec.ServiceMesh.ControlPlane.Namespace,
defaultSecretName)
Expand Down
5 changes: 2 additions & 3 deletions tests/integration/features/serverless_feature_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (
dsciv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1"
infrav1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/infrastructure/v1"
"github.com/opendatahub-io/opendatahub-operator/v2/components/kserve"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/feature"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/feature/serverless"
"github.com/opendatahub-io/opendatahub-operator/v2/tests/envtestutil"
Expand Down Expand Up @@ -184,7 +183,7 @@ var _ = Describe("Serverless feature", func() {
It("should set default value when value is empty in the DSCI", func() {
// Default value is blank -> testFeature.Spec.Serving.IngressGateway.Certificate.SecretName = ""
Expect(serverless.ServingDefaultValues(testFeature)).To(Succeed())
Expect(testFeature.Spec.KnativeCertificateSecret).To(Equal(cluster.DefaultCertificateSecretName))
Expect(testFeature.Spec.KnativeCertificateSecret).To(Equal(serverless.DefaultCertificateSecretName))
})

It("should use user value when set in the DSCI", func() {
Expand Down Expand Up @@ -262,7 +261,7 @@ var _ = Describe("Serverless feature", func() {

// then
Eventually(func() error {
secret, err := envTestClientset.CoreV1().Secrets(namespace.Name).Get(context.TODO(), cluster.DefaultCertificateSecretName, metav1.GetOptions{})
secret, err := envTestClientset.CoreV1().Secrets(namespace.Name).Get(context.TODO(), serverless.DefaultCertificateSecretName, metav1.GetOptions{})
if err != nil {
return err
}
Expand Down

0 comments on commit 114395b

Please sign in to comment.