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.

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 $HOME resolution and webhook namespace #4509

Merged
merged 4 commits into from
Nov 30, 2023
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
16 changes: 12 additions & 4 deletions cmd/single/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package single
import (
"context"
"net/http"
"os"
metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
ctrlWebhook "sigs.k8s.io/controller-runtime/pkg/webhook"

Expand Down Expand Up @@ -40,6 +41,7 @@ import (
)

const defaultNamespace = "all"
const propellerDefaultNamespace = "flyte"

func startDataCatalog(ctx context.Context, _ DataCatalog) error {
if err := datacatalogRepo.Migrate(ctx); err != nil {
Expand Down Expand Up @@ -120,7 +122,7 @@ func startPropeller(ctx context.Context, cfg Propeller) error {
SyncPeriod: &propellerCfg.DownstreamEval.Duration,
DefaultNamespaces: namespaceConfigs,
},
NewCache: func (config *rest.Config, options cache.Options) (cache.Cache, error) {
NewCache: func(config *rest.Config, options cache.Options) (cache.Cache, error) {
k8sCache, err := cache.New(config, options)
if err != nil {
return k8sCache, err
Expand All @@ -141,7 +143,7 @@ func startPropeller(ctx context.Context, cfg Propeller) error {
BindAddress: "0",
},
WebhookServer: ctrlWebhook.NewServer(ctrlWebhook.Options{
CertDir: webhookConfig.GetConfig().CertDir,
CertDir: webhookConfig.GetConfig().ExpandCertDir(),
Port: webhookConfig.GetConfig().ListenPort,
}),
}
Expand All @@ -162,7 +164,13 @@ func startPropeller(ctx context.Context, cfg Propeller) error {
return err
}
logger.Infof(childCtx, "Starting Webhook server...")
return webhookEntrypoint.Run(signals.SetupSignalHandler(childCtx), propellerCfg, webhookConfig.GetConfig(), defaultNamespace, &propellerScope, mgr)
// set default namespace for pod template store
podNamespace, found := os.LookupEnv(webhookEntrypoint.PodNamespaceEnvVar)
if !found {
podNamespace = propellerDefaultNamespace
}

return webhookEntrypoint.Run(signals.SetupSignalHandler(childCtx), propellerCfg, webhookConfig.GetConfig(), podNamespace, &propellerScope, mgr)
})
}

Expand Down Expand Up @@ -207,7 +215,7 @@ var startCmd = &cobra.Command{
for _, serviceName := range []string{otelutils.AdminClientTracer, otelutils.AdminGormTracer, otelutils.AdminServerTracer,
otelutils.BlobstoreClientTracer, otelutils.DataCatalogClientTracer, otelutils.DataCatalogGormTracer,
otelutils.DataCatalogServerTracer, otelutils.FlytePropellerTracer, otelutils.K8sClientTracer} {
if err := otelutils.RegisterTracerProvider(serviceName, otelutils.GetConfig()) ; err != nil {
if err := otelutils.RegisterTracerProvider(serviceName, otelutils.GetConfig()); err != nil {
logger.Errorf(ctx, "Failed to create otel tracer provider. %v", err)
return err
}
Expand Down
3 changes: 2 additions & 1 deletion flyteadmin/pkg/runtime/cluster_resource_provider.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package runtime

import (
"os"
"time"

"github.com/flyteorg/flyte/flyteadmin/pkg/runtime/interfaces"
Expand All @@ -21,7 +22,7 @@ var clusterResourceConfig = config.MustRegisterSection(clusterResourceKey, &inte
type ClusterResourceConfigurationProvider struct{}

func (p *ClusterResourceConfigurationProvider) GetTemplatePath() string {
return clusterResourceConfig.GetConfig().(*interfaces.ClusterResourceConfig).TemplatePath
return os.ExpandEnv(clusterResourceConfig.GetConfig().(*interfaces.ClusterResourceConfig).TemplatePath)
}

func (p *ClusterResourceConfigurationProvider) GetTemplateData() interfaces.TemplateData {
Expand Down
2 changes: 1 addition & 1 deletion flytepropeller/cmd/controller/cmd/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ func runWebhook(origContext context.Context, propellerCfg *config.Config, cfg *w
BindAddress: "0",
},
WebhookServer: ctrlWebhook.NewServer(ctrlWebhook.Options{
CertDir: cfg.CertDir,
CertDir: cfg.ExpandCertDir(),
Port: cfg.ListenPort,
}),
}
Expand Down
5 changes: 5 additions & 0 deletions flytepropeller/pkg/webhook/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import (
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
"os"

"github.com/flyteorg/flyte/flytestdlib/config"
)
Expand Down Expand Up @@ -103,6 +104,10 @@
VaultSecretManagerConfig VaultSecretManagerConfig `json:"vaultSecretManager" pflag:",Vault Secret Manager config."`
}

func (c Config) ExpandCertDir() string {
return os.ExpandEnv(c.CertDir)

Check warning on line 108 in flytepropeller/pkg/webhook/config/config.go

View check run for this annotation

Codecov / codecov/patch

flytepropeller/pkg/webhook/config/config.go#L107-L108

Added lines #L107 - L108 were not covered by tests
}

type AWSSecretManagerConfig struct {
SidecarImage string `json:"sidecarImage" pflag:",Specifies the sidecar docker image to use"`
Resources corev1.ResourceRequirements `json:"resources" pflag:"-,Specifies resource requirements for the init container."`
Expand Down
11 changes: 6 additions & 5 deletions flytepropeller/pkg/webhook/init_cert.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,21 +78,22 @@
}

if cfg.LocalCert {
if _, err := os.Stat(cfg.CertDir); os.IsNotExist(err) {
if err := os.Mkdir(cfg.CertDir, folderPerm); err != nil {
certPath := cfg.ExpandCertDir()
if _, err := os.Stat(certPath); os.IsNotExist(err) {
if err := os.Mkdir(certPath, folderPerm); err != nil {

Check warning on line 83 in flytepropeller/pkg/webhook/init_cert.go

View check run for this annotation

Codecov / codecov/patch

flytepropeller/pkg/webhook/init_cert.go#L81-L83

Added lines #L81 - L83 were not covered by tests
return err
}
}

if err := os.WriteFile(path.Join(cfg.CertDir, CaCertKey), certs.CaPEM.Bytes(), permission); err != nil {
if err := os.WriteFile(path.Join(certPath, CaCertKey), certs.CaPEM.Bytes(), permission); err != nil {

Check warning on line 88 in flytepropeller/pkg/webhook/init_cert.go

View check run for this annotation

Codecov / codecov/patch

flytepropeller/pkg/webhook/init_cert.go#L88

Added line #L88 was not covered by tests
return err
}

if err := os.WriteFile(path.Join(cfg.CertDir, ServerCertKey), certs.ServerPEM.Bytes(), permission); err != nil {
if err := os.WriteFile(path.Join(certPath, ServerCertKey), certs.ServerPEM.Bytes(), permission); err != nil {

Check warning on line 92 in flytepropeller/pkg/webhook/init_cert.go

View check run for this annotation

Codecov / codecov/patch

flytepropeller/pkg/webhook/init_cert.go#L92

Added line #L92 was not covered by tests
return err
}

if err := os.WriteFile(path.Join(cfg.CertDir, ServerCertPrivateKey), certs.PrivateKeyPEM.Bytes(), permission); err != nil {
if err := os.WriteFile(path.Join(certPath, ServerCertPrivateKey), certs.PrivateKeyPEM.Bytes(), permission); err != nil {

Check warning on line 96 in flytepropeller/pkg/webhook/init_cert.go

View check run for this annotation

Codecov / codecov/patch

flytepropeller/pkg/webhook/init_cert.go#L96

Added line #L96 was not covered by tests
return err
}
}
Expand Down
16 changes: 8 additions & 8 deletions flytepropeller/pkg/webhook/pod.go
Original file line number Diff line number Diff line change
@@ -1,19 +1,20 @@
// The PodMutator is a controller-runtime webhook that intercepts Pod Creation events and mutates them. Currently, there
// is only one registered Mutator, that's the SecretsMutator. It works as follows:
// Package webhook container PodMutator. It's a controller-runtime webhook that intercepts Pod Creation events and
// mutates them. Currently, there is only one registered Mutator, that's the SecretsMutator. It works as follows:
//
// - The Webhook only works on Pods. If propeller/plugins launch a resource outside of K8s (or in a separate k8s
// - The Webhook only works on Pods. If propeller/plugins launch a resource outside K8s (or in a separate k8s
// cluster), it's the responsibility of the plugin to correctly pass secret injection information.
// - When a k8s-plugin builds a resource, propeller's PluginManager will automatically inject a label `inject-flyte
// -secrets: true` and serialize the secret injection information into the annotations.
// - If a plugin does not use the K8sPlugin interface, it's its responsibility to pass secret injection information.
// - If a k8s plugin creates a CRD that launches other Pods (e.g. Spark/PyTorch... etc.), it's its responsibility to
// make sure the labels/annotations set on the CRD by PluginManager are propagated to those launched Pods. This
// ensures secret injection happens no matter how many levels of indirections there are.
// - The Webhook expects 'inject-flyte-secrets: true' as a label on the Pod. Otherwise it won't listen/observe that pod.
// - The Webhook expects 'inject-flyte-secrets: true' as a label on the Pod. Otherwise, it won't listen/observe that
// pod.
// - Once it intercepts the admission request, it goes over all registered Mutators and invoke them in the order they
// are registered as. If a Mutator fails and it's marked as `required`, the operation will fail and the admission
// are registered as. If a Mutator fails, and it's marked as `required`, the operation will fail and the admission
// will be rejected.
// - The SecretsMutator will attempt to lookup the requested secret from the process environment. If the secret is
// - The SecretsMutator will attempt to look up the requested secret from the process environment. If the secret is
// already mounted, it'll inject it as plain-text into the Pod Spec (Less secure).
// - If it's not found in the environment it'll, instead, fallback to the enabled Secrets Injector (K8s, Confidant,
// Vault... etc.).
Expand All @@ -30,7 +31,6 @@ import (
"context"
"encoding/json"
"fmt"
"io/ioutil"
"k8s.io/apimachinery/pkg/runtime"
"net/http"
"os"
Expand Down Expand Up @@ -148,7 +148,7 @@ func generateMutatePath(gvk schema.GroupVersionKind) string {
}

func (pm PodMutator) CreateMutationWebhookConfiguration(namespace string) (*admissionregistrationv1.MutatingWebhookConfiguration, error) {
caBytes, err := ioutil.ReadFile(filepath.Join(pm.cfg.CertDir, "ca.crt"))
caBytes, err := os.ReadFile(filepath.Join(pm.cfg.ExpandCertDir(), "ca.crt"))
if err != nil {
// ca.crt is optional. If not provided, API Server will assume the webhook is serving SSL using a certificate
// issued by a known Cert Authority.
Expand Down
12 changes: 2 additions & 10 deletions rsts/community/contribute.rst
Original file line number Diff line number Diff line change
Expand Up @@ -389,15 +389,7 @@ that integrates all Flyte components into a single binary.
go mod tidy
make compile

# Step3: Edit the config file: ./flyte-single-binary-local.yaml.
# Replace occurrences of $HOME with the actual path of your home directory.
sedi=(-i)
case "$(uname)" in
Darwin*) sedi=(-i "")
esac
sed "${sedi[@]}" -e "s|\$HOME|${HOME}|g" flyte-single-binary-local.yaml

# Step 4: Prepare a namespace template for the cluster resource controller.
# Step3: Prepare a namespace template for the cluster resource controller.
# The configuration file "flyte-single-binary-local.yaml" has an entry named cluster_resources.templatePath.
# This entry needs to direct to a directory containing the templates for the cluster resource controller to use.
# We will now create a simple template that allows the automatic creation of required namespaces for projects.
Expand All @@ -409,7 +401,7 @@ that integrates all Flyte components into a single binary.
metadata:
name: '{{ namespace }}'" > $HOME/.flyte/cluster-resource-templates/namespace.yaml

# Step5: Running the single binary.
# Step4: Running the single binary.
# The POD_NAMESPACE environment variable is necessary for the webhook to function correctly.
# You may encounter an error due to `ERROR: duplicate key value violates unique constraint`. Running the command again will solve the problem.
POD_NAMESPACE=flyte ./flyte start --config flyte-single-binary-local.yaml
Expand Down
Loading