Skip to content
This repository has been archived by the owner on Jul 11, 2023. It is now read-only.

Remove crd conversion webhook #5065

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 0 additions & 12 deletions charts/osm/templates/osm-bootstrap-deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -33,18 +33,6 @@ spec:
{{- if not (.Capabilities.APIVersions.Has "security.openshift.io/v1") }}
{{- include "restricted.securityContext" . | nindent 6 }}
{{- end }}
initContainers:
- name: init-osm-bootstrap
image: "{{ include "osmCRDs.image" . }}"
imagePullPolicy: {{ .Values.osm.image.pullPolicy }}
command:
- sh
- -c
- >
kubectl apply -f /osm-crds;
{{- if .Values.osm.enableReconciler }}
kubectl label -f /osm-crds openservicemesh.io/reconcile=true --overwrite;
{{- end }}
containers:
- name: osm-bootstrap
image: "{{ include "osmBootstrap.image" . }}"
Expand Down
2 changes: 2 additions & 0 deletions cmd/osm-bootstrap/crds/config_mesh_root_certificate.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ spec:
- mrc
singular: meshrootcertificate
plural: meshrootcertificates
conversion:
strategy: None
versions:
- name: v1alpha2
served: true
Expand Down
2 changes: 2 additions & 0 deletions cmd/osm-bootstrap/crds/config_meshconfig.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ spec:
- meshconfig
singular: meshconfig
plural: meshconfigs
conversion:
strategy: None
versions:
- name: v1alpha2
served: true
Expand Down
2 changes: 2 additions & 0 deletions cmd/osm-bootstrap/crds/policy_egress.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ spec:
- egress
singular: egress
plural: egresses
conversion:
strategy: None
versions:
- name: v1alpha1
served: true
Expand Down
2 changes: 2 additions & 0 deletions cmd/osm-bootstrap/crds/policy_ingress_backend.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ spec:
- ingressbackend
singular: ingressbackend
plural: ingressbackends
conversion:
strategy: None
versions:
- name: v1alpha1
served: true
Expand Down
2 changes: 2 additions & 0 deletions cmd/osm-bootstrap/crds/policy_retry.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ spec:
- retry
singular: retry
plural: retries
conversion:
strategy: None
versions:
- name: v1alpha1
served: true
Expand Down
2 changes: 2 additions & 0 deletions cmd/osm-bootstrap/crds/policy_upstream_traffic_setting.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ spec:
- upstreamtrafficsetting
singular: upstreamtrafficsetting
plural: upstreamtrafficsettings
conversion:
strategy: None
versions:
- name: v1alpha1
served: true
Expand Down
2 changes: 2 additions & 0 deletions cmd/osm-bootstrap/crds/smi_http_route_group.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ spec:
- htr
plural: httproutegroups
singular: httproutegroup
conversion:
strategy: None
versions:
- name: v1alpha4
served: true
Expand Down
2 changes: 2 additions & 0 deletions cmd/osm-bootstrap/crds/smi_tcp_route.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ spec:
- tr
plural: tcproutes
singular: tcproute
conversion:
strategy: None
versions:
- name: v1alpha4
served: true
Expand Down
2 changes: 2 additions & 0 deletions cmd/osm-bootstrap/crds/smi_traffic_access.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ spec:
- tt
plural: traffictargets
singular: traffictarget
conversion:
strategy: None
versions:
- name: v1alpha3
served: true
Expand Down
2 changes: 2 additions & 0 deletions cmd/osm-bootstrap/crds/smi_traffic_split.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ spec:
- ts
plural: trafficsplits
singular: trafficsplit
conversion:
strategy: None
versions:
- name: v1alpha4
served: false
Expand Down
138 changes: 71 additions & 67 deletions cmd/osm-bootstrap/osm-bootstrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,36 +11,34 @@ import (
"fmt"
"net/http"
"os"
"time"
"path/filepath"
"strconv"

"github.com/spf13/pflag"
admissionv1 "k8s.io/api/admissionregistration/v1"
corev1 "k8s.io/api/core/v1"
apiv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
clientset "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset"
apiclient "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/serializer"
"k8s.io/client-go/kubernetes"
clientgoscheme "k8s.io/client-go/kubernetes/scheme"
"k8s.io/client-go/tools/clientcmd"
"k8s.io/kubectl/pkg/util"

configv1alpha2 "github.com/openservicemesh/osm/pkg/apis/config/v1alpha2"
"github.com/openservicemesh/osm/pkg/certificate"
configClientset "github.com/openservicemesh/osm/pkg/gen/client/config/clientset/versioned"
"github.com/openservicemesh/osm/pkg/health"
"github.com/openservicemesh/osm/pkg/k8s"

"github.com/openservicemesh/osm/pkg/certificate/providers"
"github.com/openservicemesh/osm/pkg/constants"
"github.com/openservicemesh/osm/pkg/crdconversion"
"github.com/openservicemesh/osm/pkg/httpserver"
"github.com/openservicemesh/osm/pkg/k8s/events"
"github.com/openservicemesh/osm/pkg/k8s/informers"
"github.com/openservicemesh/osm/pkg/logger"
"github.com/openservicemesh/osm/pkg/messaging"
"github.com/openservicemesh/osm/pkg/metricsstore"
"github.com/openservicemesh/osm/pkg/reconciler"
"github.com/openservicemesh/osm/pkg/signals"
Expand Down Expand Up @@ -68,7 +66,6 @@ var (
certProviderKind string
enableMeshRootCertificate bool

tresorOptions providers.TresorOptions
vaultOptions providers.VaultOptions
certManagerOptions providers.CertManagerOptions

Expand Down Expand Up @@ -124,21 +121,6 @@ func init() {
_ = admissionv1.AddToScheme(scheme)
}

// TODO(#4502): This function can be deleted once we get rid of cert options.
func getCertOptions() (providers.Options, error) {
switch providers.Kind(certProviderKind) {
case providers.TresorKind:
tresorOptions.SecretName = caBundleSecretName
return tresorOptions, nil
case providers.VaultKind:
vaultOptions.VaultTokenSecretNamespace = osmNamespace
return vaultOptions, nil
case providers.CertManagerKind:
return certManagerOptions, nil
}
return nil, fmt.Errorf("unknown certificate provider kind: %s", certProviderKind)
}

func main() {
log.Info().Msgf("Starting osm-bootstrap %s; %s; %s", version.Version, version.GitCommit, version.BuildDate)
if err := parseFlags(); err != nil {
Expand Down Expand Up @@ -175,6 +157,8 @@ func main() {
namespace: osmNamespace,
}

applyOrUpdateCRDs(crdClient)

err = bootstrap.ensureMeshConfig()
if err != nil {
log.Fatal().Err(err).Msgf("Error setting up default MeshConfig %s from ConfigMap %s", meshConfigName, presetMeshConfigName)
Expand All @@ -194,7 +178,7 @@ func main() {
log.Fatal().Err(err).Msg("Error initializing Kubernetes events recorder")
}

ctx, cancel := context.WithCancel(context.Background())
_, cancel := context.WithCancel(context.Background())
defer cancel()
stop := signals.RegisterExitHandlers(cancel)

Expand All @@ -203,49 +187,22 @@ func main() {
metricsstore.DefaultMetricsStore.ErrCodeCounter,
metricsstore.DefaultMetricsStore.HTTPResponseTotal,
nshankar13 marked this conversation as resolved.
Show resolved Hide resolved
metricsstore.DefaultMetricsStore.HTTPResponseDuration,
metricsstore.DefaultMetricsStore.ConversionWebhookResourceTotal,
metricsstore.DefaultMetricsStore.ReconciliationTotal,
)

msgBroker := messaging.NewBroker(stop)

informerCollection, err := informers.NewInformerCollection(meshName, stop,
informers.WithKubeClient(kubeClient),
informers.WithConfigClient(configClient, osmMeshConfigName, osmNamespace),
)

if err != nil {
events.GenericEventRecorder().FatalEvent(err, events.InitializationError, "Error creating informer collection")
}

certOpts, err := getCertOptions()
if err != nil {
log.Fatal().Err(err).Msg("Error getting certificate options")
}

k8sClient := k8s.NewClient(osmNamespace, osmMeshConfigName, informerCollection, nil, msgBroker)

var certManager *certificate.Manager
if enableMeshRootCertificate {
certManager, err = providers.NewCertificateManagerFromMRC(ctx, kubeClient, kubeConfig, osmNamespace, certOpts, k8sClient, informerCollection, 5*time.Second)
if err != nil {
events.GenericEventRecorder().FatalEvent(err, events.InvalidCertificateManager,
"Error initializing certificate manager of kind %s from MRC", certProviderKind)
}
} else {
certManager, err = providers.NewCertificateManager(ctx, kubeClient, kubeConfig, osmNamespace, certOpts, k8sClient, 5*time.Second, trustDomain)
version.SetMetric()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you remove the reconciler as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@steeling looks like we also add the reconcile label to the mutatingwebhook and validatingwebhook, should we handle the reconciler removal in a follow up PR? Or will removing the conversionwebhook while keeping the reconciler introduce bugs?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should delete it for all. it's broken on upgrades in that an old reconciler will stomp over any changes we try to make

/*
* Initialize osm-bootstrap's HTTP server
*/
if enableReconciler {
log.Info().Msgf("OSM reconciler enabled for custom resource definitions")
err = reconciler.NewReconcilerClient(kubeClient, apiServerClient, meshName, osmVersion, stop, reconciler.CrdInformerKey)
if err != nil {
events.GenericEventRecorder().FatalEvent(err, events.InvalidCertificateManager,
"Error initializing certificate manager of kind %s", certProviderKind)
events.GenericEventRecorder().FatalEvent(err, events.InitializationError, "Error creating reconciler client for custom resource definitions")
log.Fatal().Err(err).Msgf("Failed to create reconcile client for custom resource definitions")
}
}

// Initialize the crd conversion webhook server to support the conversion of OSM's CRDs
if err := crdconversion.NewConversionWebhook(ctx, crdClient, certManager, osmNamespace); err != nil {
events.GenericEventRecorder().FatalEvent(err, events.InitializationError, fmt.Sprintf("Error creating crd conversion webhook: %s", err))
}

version.SetMetric()
/*
* Initialize osm-bootstrap's HTTP server
*/
Expand All @@ -263,19 +220,66 @@ func main() {
log.Fatal().Err(err).Msgf("Failed to start OSM metrics/probes HTTP server")
}

if enableReconciler {
log.Info().Msgf("OSM reconciler enabled for custom resource definitions")
err = reconciler.NewReconcilerClient(kubeClient, apiServerClient, meshName, osmVersion, stop, reconciler.CrdInformerKey)
if err != nil {
events.GenericEventRecorder().FatalEvent(err, events.InitializationError, "Error creating reconciler client for custom resource definitions")
}
}

<-stop
cancel()
log.Info().Msgf("Stopping osm-bootstrap %s; %s; %s", version.Version, version.GitCommit, version.BuildDate)
}

func applyOrUpdateCRDs(crdClient *apiclient.ApiextensionsV1Client) {
crdFiles, err := filepath.Glob("/osm-crds/*.yaml")

if err != nil {
log.Fatal().Err(err).Msgf("error reading files from /osm-crds")
}

scheme = runtime.NewScheme()
codecs := serializer.NewCodecFactory(scheme)
decode := codecs.UniversalDeserializer().Decode

for _, file := range crdFiles {
yaml, err := os.ReadFile(filepath.Clean(file))
if err != nil {
log.Fatal().Err(err).Msgf("Error reading CRD file %s", file)
}

crd := &apiv1.CustomResourceDefinition{}
_, _, err = decode(yaml, nil, crd)
if err != nil {
log.Fatal().Err(err).Msgf("Error decoding CRD file %s", file)
}

crd.Labels[constants.ReconcileLabel] = strconv.FormatBool(enableReconciler)

crdExisting, err := crdClient.CustomResourceDefinitions().Get(context.Background(), crd.Name, metav1.GetOptions{})
if err != nil && !apierrors.IsNotFound(err) {
log.Fatal().Err(err).Msgf("error getting CRD %s", crd.Name)
}

if apierrors.IsNotFound(err) {
log.Info().Msgf("crds %s not found, creating CRD", crd.Name)
if err := util.CreateApplyAnnotation(crd, unstructured.UnstructuredJSONScheme); err != nil {
log.Fatal().Err(err).Msgf("Error applying annotation to CRD %s", crd.Name)
}
if _, err = crdClient.CustomResourceDefinitions().Create(context.Background(), crd, metav1.CreateOptions{}); err != nil {
log.Fatal().Err(err).Msgf("Error creating crd : %s", crd.Name)
}
log.Info().Msgf("Successfully created crd: %s", crd.Name)
} else {
log.Info().Msgf("Patching conversion webhook configuration for crd: %s, setting to \"None\"", crd.Name)

crdExisting.Labels[constants.ReconcileLabel] = strconv.FormatBool(enableReconciler)
crdExisting.Spec = crd.Spec
crdExisting.Spec.Conversion = &apiv1.CustomResourceConversion{
Strategy: apiv1.NoneConverter,
}
if _, err = crdClient.CustomResourceDefinitions().Update(context.Background(), crdExisting, metav1.UpdateOptions{}); err != nil {
log.Fatal().Err(err).Msgf("Error updating conversion webhook configuration for crd : %s", crd.Name)
}
log.Info().Msgf("successfully set conversion webhook configuration for crd : %s to \"None\"", crd.Name)
}
}
}

func (b *bootstrap) createDefaultMeshConfig() error {
// find presets config map to build the default MeshConfig from that
presetsConfigMap, err := b.kubeClient.CoreV1().ConfigMaps(b.namespace).Get(context.TODO(), presetMeshConfigName, metav1.GetOptions{})
Expand Down
1 change: 1 addition & 0 deletions dockerfiles/Dockerfile.osm-bootstrap
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,4 @@ RUN --mount=type=cache,target=/root/.cache/go-build \
FROM $FINAL_BASE_IMAGE
ENV GOFIPS=1
COPY --from=builder /osm/osm-bootstrap /
COPY ./cmd/osm-bootstrap/crds /osm-crds/
1 change: 0 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ require (
github.com/mholt/archiver/v3 v3.5.1
github.com/mitchellh/gox v1.0.1
github.com/mitchellh/hashstructure/v2 v2.0.1
github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822
github.com/norwoodj/helm-docs v1.4.0
github.com/olekukonko/tablewriter v0.0.5
github.com/onsi/ginkgo v1.16.5
Expand Down
1 change: 0 additions & 1 deletion go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -1401,7 +1401,6 @@ github.com/mozilla/tls-observatory v0.0.0-20200317151703-4fa42e1c2dee/go.mod h1:
github.com/mrunalp/fileutils v0.5.0/go.mod h1:M1WthSahJixYnrXQl/DFQuteStB1weuxD2QJNHXfbSQ=
github.com/munnerz/crd-schema-fuzz v1.0.0/go.mod h1:4z/rcm37JxUkSsExFcLL6ZIT1SgDRdLiu7qq1evdVS0=
github.com/munnerz/goautoneg v0.0.0-20120707110453-a547fc61f48d/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ=
github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 h1:C3w9PqII01/Oq1c1nUAm88MOHcQC9l5mIlSMApZMrHA=
github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ=
github.com/mwitkow/go-conntrack v0.0.0-20161129095857-cc309e4a2223/go.mod h1:qRWi+5nqEBWmkhHvq77mSJWrCKwh8bxhgT7d/eI7P4U=
github.com/mwitkow/go-conntrack v0.0.0-20190716064945-2f068394615f h1:KUppIJq7/+SVif2QVs3tOP0zanoHgBEVAwHxUSIzRqU=
Expand Down
3 changes: 0 additions & 3 deletions pkg/constants/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,6 @@ const (

// OSMMeshConfig is the name of the OSM MeshConfig
OSMMeshConfig = "osm-mesh-config"

// CRDConversionWebhookPort is the port of the CRD conversion webhook service
CRDConversionWebhookPort = 9443
)

// HealthProbe constants
Expand Down
Loading