Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix 1.22 Webhook Create by ensuring v1 is used #3736

Merged
merged 1 commit into from
Nov 13, 2021
Merged

Fix 1.22 Webhook Create by ensuring v1 is used #3736

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion operator/utils/k8s/configmap.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ type ConfigmapCreator struct {
func NewConfigmapCreator(client kubernetes.Interface, logger logr.Logger, scheme *runtime.Scheme) *ConfigmapCreator {
return &ConfigmapCreator{
clientset: client,
logger: logger,
logger: logger.WithName("ConfigMapCreator"),
scheme: scheme,
}
}
Expand All @@ -43,6 +43,7 @@ func (cc *ConfigmapCreator) CreateConfigmap(ctx context.Context, rawYaml []byte,
// add ownership
err = ctrl.SetControllerReference(owner, &cm, cc.scheme)
if err != nil {
cc.logger.Error(err, "Failed to set controller reference on configmap")
return err
}

Expand Down
11 changes: 10 additions & 1 deletion operator/utils/k8s/crd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ func NewCrdCreator(ctx context.Context, apiExtensionsClient apiextensionsclient.
return &CrdCreator{
apiExtensionsClient: apiExtensionsClient,
discoveryClient: discoveryClient,
logger: logger,
logger: logger.WithName("CRDCreator"),
ctx: ctx,
}
}
Expand All @@ -43,6 +43,7 @@ func (cc *CrdCreator) createCRDV1beta1(rawYaml []byte) (*v1beta1.CustomResourceD
crd := v1beta1.CustomResourceDefinition{}
err := yaml.Unmarshal(rawYaml, &crd)
if err != nil {
cc.logger.Error(err, "Failed to unmarshall v1beta1 CRD")
return nil, err
}
client := cc.apiExtensionsClient.ApiextensionsV1beta1().CustomResourceDefinitions()
Expand All @@ -53,6 +54,7 @@ func (cc *CrdCreator) createCRDV1(rawYaml []byte) (*extensionsv1.CustomResourceD
crd := extensionsv1.CustomResourceDefinition{}
err := yaml.Unmarshal(rawYaml, &crd)
if err != nil {
cc.logger.Error(err, "Failed to unmarshall V1 CRD")
return nil, err
}
client := cc.apiExtensionsClient.ApiextensionsV1().CustomResourceDefinitions()
Expand All @@ -68,10 +70,12 @@ func (cc *CrdCreator) findOrCreateCRDV1beta1(rawYaml []byte) (v1.Object, error)
cc.logger.Info("CRD v1beta1 not found - trying to create")
crd, err = cc.createCRDV1beta1(rawYaml)
if err != nil {
cc.logger.Error(err, "Failed to create v1beta1 CRD")
return nil, err
}
cc.logger.Info("CRD v1beta1 created")
} else {
cc.logger.Error(err, "Failed finding v1beta1 crd")
return nil, err
}
} else {
Expand All @@ -89,10 +93,12 @@ func (cc *CrdCreator) findOrCreateCRDV1(rawYaml []byte) (*extensionsv1.CustomRes
cc.logger.Info("CRD V1 not found - trying to create")
crd, err = cc.createCRDV1(rawYaml)
if err != nil {
cc.logger.Error(err, "Failed to create v1 CRD")
return nil, err
}
cc.logger.Info("CRD V1 created")
} else {
cc.logger.Error(err, "Failed finding V1 CRD")
return nil, err
}
} else {
Expand All @@ -104,14 +110,17 @@ func (cc *CrdCreator) findOrCreateCRDV1(rawYaml []byte) (*extensionsv1.CustomRes
func (cc *CrdCreator) findOrCreateCRD(rawYamlv1 []byte, rawYamlv1beta1 []byte) (v1.Object, error) {
serverVersion, err := GetServerVersion(cc.discoveryClient, cc.logger)
if err != nil {
cc.logger.Error(err, "Failed to get version from cluster")
return nil, err
}
v, err := semver.NewVersion(serverVersion)
if err != nil {
cc.logger.Error(err, "Failed to create semver Version")
return nil, err
}
c, err := semver.NewConstraint(">= 1.18.0")
if err != nil {
cc.logger.Error(err, "Failed applying constraint to check greater than 1.18.0 cluster")
return nil, err
}
check := c.Check(v)
Expand Down
42 changes: 29 additions & 13 deletions operator/utils/k8s/initializer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,35 +48,42 @@ func InitializeOperator(ctx context.Context, config *rest.Config, namespace stri

apiExtensionClient, err := apiextensionsclient.NewForConfig(config)
if err != nil {
logger.Error(err, "Failed to create apiextensionsClient")
return err
}

discoveryClient, err := discovery.NewDiscoveryClientForConfig(config)
if err != nil {
logger.Error(err, "Failed to create discoveryClient")
return err
}

crdCreator := NewCrdCreator(ctx, apiExtensionClient, discoveryClient, logger)
bytesV1Beta1, err := LoadBytesFromFile(ResourceFolder, CRDFilenameV1Beta1)
if err != nil {
logger.Error(err, "Failed to find crd v1beta1", "resourcefolder", ResourceFolder, "filename", CRDFilenameV1Beta1)
return err
}
bytesV1, err := LoadBytesFromFile(ResourceFolder, CRDFilenameV1)
if err != nil {
logger.Error(err, "Failed to find crd v1", "resourcefolder", ResourceFolder, "filename", CRDFilenameV1)
return err
}
crd, err := crdCreator.findOrCreateCRD(bytesV1, bytesV1Beta1)
if err != nil {
logger.Error(err, "Failed to create CRD")
return err
}

clientset, err := kubernetes.NewForConfig(config)
if err != nil {
logger.Error(err, "Failed to create clientset")
return err
}

dep, err := findMyDeployment(ctx, clientset, namespace)
if err != nil {
logger.Error(err, "Failed to find deployment")
return err
}

Expand All @@ -85,78 +92,87 @@ func InitializeOperator(ctx context.Context, config *rest.Config, namespace stri
host2 := fmt.Sprintf("seldon-webhook-service.%s.svc", namespace)
certs, err := certSetup([]string{host1, host2})
if err != nil {
logger.Error(err, "Failed to create certs")
return err
}

// Create webhooks
wc, err := NewWebhookCreator(clientset, certs, logger, scheme)
if err != nil {
return err
}

//Delete mutating webhook if existing
err = wc.DeleteMutatingWebhook(ctx)
if err != nil {
return err
}
wc := NewWebhookCreator(clientset, certs, logger, scheme)

//Create/Update Validating Webhook
bytes, err := LoadBytesFromFile(ResourceFolder, ValidatingWebhookFilename)
if err != nil {
logger.Error(err, "Failed to find webhook file", "resourcefolder", ResourceFolder, "filename", ValidatingWebhookFilename)
return err
}
err = wc.CreateValidatingWebhookConfigurationFromFile(ctx, bytes, namespace, crd, watchNamespace)
if err != nil {
logger.Error(err, "Failed to create validating webhook")
return err
}

//Create/Update Webhook Service
bytes, err = LoadBytesFromFile(ResourceFolder, ServiceFilename)
if err != nil {
logger.Error(err, "Failed to find webhook service", "resourcefolder", ResourceFolder, "filename", ServiceFilename)
return err
}
err = wc.CreateWebhookServiceFromFile(ctx, bytes, namespace, dep)
if err != nil {
logger.Error(err, "Failed to create webhook service")
return err
}

//Create Configmap
cc := NewConfigmapCreator(clientset, logger, scheme)
bytes, err = LoadBytesFromFile(ResourceFolder, ConfigMapFilename)
if err != nil {
logger.Error(err, "Failed to find configmap", "resourcefolder", ResourceFolder, "filename", ConfigMapFilename)
return err
}
err = cc.CreateConfigmap(ctx, bytes, namespace, dep)
if err != nil {
logger.Error(err, "Failed to create webhook")
return err
}

// Create cert files
createCertFiles(certs, logger)
err = createCertFiles(certs, logger)
if err != nil {
logger.Error(err, "Failed to create crts files")
return err
}

return nil
}

func createCertFiles(certs *Cert, logger logr.Logger) error {
//Save certs to filesystem
os.MkdirAll(CertsFolder, os.ModePerm)
err := os.MkdirAll(CertsFolder, os.ModePerm)
if err != nil {
logger.Error(err, "Failed to create folder", "folder", CertsFolder)
return err
}

filename := fmt.Sprintf("%s/%s", CertsFolder, CertsTLSCa)
logger.Info("Creating ", "filename", filename)
err := ioutil.WriteFile(filename, []byte(certs.caPEM), 0600)
err = ioutil.WriteFile(filename, []byte(certs.caPEM), 0600)
if err != nil {
logger.Error(err, "failed to create cert file", "filename", filename)
return err
}
filename = fmt.Sprintf("%s/%s", CertsFolder, CertsTLSKey)
logger.Info("Creating ", "filename", filename)
err = ioutil.WriteFile(filename, []byte(certs.privKeyPEM), 0600)
if err != nil {
logger.Error(err, "failed to create cert file", "filename", filename)
return err
}
filename = fmt.Sprintf("%s/%s", CertsFolder, CertsTLSCrt)
logger.Info("Creating ", "filename", filename)
err = ioutil.WriteFile(filename, []byte(certs.certificatePEM), 0600)
if err != nil {
logger.Error(err, "failed to create cert file", "filename", filename)
return err
}

Expand Down
1 change: 1 addition & 0 deletions operator/utils/k8s/utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ const DefaultMinorVersion = 18
func GetServerVersion(client discovery.DiscoveryInterface, logger logr.Logger) (string, error) {
serverVersion, err := client.ServerVersion()
if err != nil {
logger.Error(err, "Failed to get server version")
return "", err
}
logger.Info("Server version", "Major", serverVersion.Major, "Minor", serverVersion.Minor)
Expand Down
49 changes: 23 additions & 26 deletions operator/utils/k8s/webhook.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import (
"context"
"github.com/ghodss/yaml"
"github.com/go-logr/logr"
"k8s.io/api/admissionregistration/v1beta1"
adv1 "k8s.io/api/admissionregistration/v1"
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
Expand All @@ -23,48 +23,35 @@ type WebhookCreator struct {
scheme *runtime.Scheme
}

func NewWebhookCreator(client kubernetes.Interface, certs *Cert, logger logr.Logger, scheme *runtime.Scheme) (*WebhookCreator, error) {
func NewWebhookCreator(client kubernetes.Interface, certs *Cert, logger logr.Logger, scheme *runtime.Scheme) *WebhookCreator {
return &WebhookCreator{
clientset: client,
certs: certs,
logger: logger,
logger: logger.WithName("WebhookCreator"),
scheme: scheme,
}, nil
}

func (wc *WebhookCreator) DeleteMutatingWebhook(ctx context.Context) error {
client := wc.clientset.AdmissionregistrationV1beta1().MutatingWebhookConfigurations()

// Try to delete clusterwide webhook config if available (older versions of seldon core)
_, err := client.Get(ctx, MutatingWebhookName, v1.GetOptions{})
if err != nil && errors.IsNotFound(err) {
wc.logger.Info("existing clusterwide mwc not found", "name", MutatingWebhookName)
} else {
client.Delete(ctx, MutatingWebhookName, v1.DeleteOptions{})
if err != nil {
return err
}
wc.logger.Info("Deleted clusterwide mwc", "name", MutatingWebhookName)
}

return nil
}

func (wc *WebhookCreator) CreateValidatingWebhookConfigurationFromFile(ctx context.Context, rawYaml []byte, namespace string, owner v1.Object, watchNamespace bool) error {
vwc := v1beta1.ValidatingWebhookConfiguration{}
vwc := adv1.ValidatingWebhookConfiguration{}
err := yaml.Unmarshal(rawYaml, &vwc)
if err != nil {
wc.logger.Error(err, "Failed to unmarshall validating webhook")
return err
}

// create or update via client
client := wc.clientset.AdmissionregistrationV1beta1().ValidatingWebhookConfigurations()
client := wc.clientset.AdmissionregistrationV1().ValidatingWebhookConfigurations()

if watchNamespace {
// Try to delete clusterwide webhook config if available
_, err := client.Get(ctx, vwc.Name, v1.GetOptions{})
if err != nil && errors.IsNotFound(err) {
wc.logger.Info("existing clusterwide vwc not found", "name", vwc.Name)
if err != nil {
if errors.IsNotFound(err) {
wc.logger.Info("existing clusterwide vwc not found", "name", vwc.Name)
} else {
wc.logger.Error(err, "Ignoring error trying to get existing validating webhook")
}
} else {
client.Delete(ctx, vwc.Name, v1.DeleteOptions{})
if err != nil {
Expand All @@ -91,17 +78,24 @@ func (wc *WebhookCreator) CreateValidatingWebhookConfigurationFromFile(ctx conte
// add ownership
err = ctrl.SetControllerReference(owner, &vwc, wc.scheme)
if err != nil {
wc.logger.Error(err, "Failed to set owner reference on validating webhook")
return err
}

found, err := client.Get(ctx, vwc.Name, v1.GetOptions{})
if err != nil && errors.IsNotFound(err) {
wc.logger.Info("Creating validating webhook")
_, err = client.Create(ctx, &vwc, v1.CreateOptions{})
if err != nil {
wc.logger.Error(err, "Failed to create validating webhook")
}
} else if err == nil {
wc.logger.Info("Updating validating webhook")
found.Webhooks = vwc.Webhooks
_, err = client.Update(ctx, found, v1.UpdateOptions{})
if err != nil {
wc.logger.Error(err, "Failed to update validating webhook")
}
return err
}
return err
Expand Down Expand Up @@ -135,9 +129,12 @@ func (wc *WebhookCreator) CreateWebhookServiceFromFile(ctx context.Context, rawY
if err != nil && errors.IsNotFound(err) {
wc.logger.Info("Creating webhook svc")
_, err = client.Create(ctx, &svc, v1.CreateOptions{})
if err != nil {
wc.logger.Error(err, "Failed to update webhook svc")
}
} else if err == nil {
wc.logger.Info("Webhook svc exists won't update - need a patch in future")
return err
return nil
}
return err
}
13 changes: 5 additions & 8 deletions operator/utils/k8s/webhook_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,11 +110,10 @@ func TestValidatingWebhookCreate(t *testing.T) {
hosts := []string{"seldon-webhook-service.seldon-system", "seldon-webhook-service.seldon-system.svc"}
certs, err := certSetup(hosts)
g.Expect(err).To(BeNil())
wc, err := NewWebhookCreator(client, certs, ctrl.Log, scheme)
g.Expect(err).To(BeNil())
wc := NewWebhookCreator(client, certs, ctrl.Log, scheme)
err = wc.CreateValidatingWebhookConfigurationFromFile(context.TODO(), bytes, TestNamespace, crd, false)
g.Expect(err).To(BeNil())
_, err = client.AdmissionregistrationV1beta1().ValidatingWebhookConfigurations().Get(context.TODO(), "seldon-validating-webhook", metav1.GetOptions{})
_, err = client.AdmissionregistrationV1().ValidatingWebhookConfigurations().Get(context.TODO(), "seldon-validating-webhook", metav1.GetOptions{})
g.Expect(err).To(BeNil())
}

Expand All @@ -130,11 +129,10 @@ func TestValidatingWebhookCreateNamespaced(t *testing.T) {
hosts := []string{"seldon-webhook-service.seldon-system", "seldon-webhook-service.seldon-system.svc"}
certs, err := certSetup(hosts)
g.Expect(err).To(BeNil())
wc, err := NewWebhookCreator(client, certs, ctrl.Log, scheme)
g.Expect(err).To(BeNil())
wc := NewWebhookCreator(client, certs, ctrl.Log, scheme)
err = wc.CreateValidatingWebhookConfigurationFromFile(context.TODO(), bytes, TestNamespace, crd, true)
g.Expect(err).To(BeNil())
_, err = client.AdmissionregistrationV1beta1().ValidatingWebhookConfigurations().Get(context.TODO(), "seldon-validating-webhook-"+TestNamespace, metav1.GetOptions{})
_, err = client.AdmissionregistrationV1().ValidatingWebhookConfigurations().Get(context.TODO(), "seldon-validating-webhook-"+TestNamespace, metav1.GetOptions{})
g.Expect(err).To(BeNil())
}

Expand All @@ -149,8 +147,7 @@ func TestWebHookSvcCreate(t *testing.T) {
hosts := []string{"seldon-webhook-service.seldon-system", "seldon-webhook-service.seldon-system.svc"}
certs, err := certSetup(hosts)
g.Expect(err).To(BeNil())
wc, err := NewWebhookCreator(client, certs, ctrl.Log, scheme)
g.Expect(err).To(BeNil())
wc := NewWebhookCreator(client, certs, ctrl.Log, scheme)
err = wc.CreateWebhookServiceFromFile(context.TODO(), bytes, TestNamespace, dep)
g.Expect(err).To(BeNil())
}