Remove unused code in webhooks.go (#566)

* Remove unused code in webhooks

* Fix issue where watch goroutine was failing after test completion

Author: kushnaidu

pkg/apiserver/webhooks.go

@@ -50,13 +50,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
-type WebhookType string
-
-const (
-	WebhookTypeService           WebhookType = "service"
-	WebhookTypeUrl               WebhookType = "url"
-	repositoryValidationEndpoint             = "/validate-repository"
-)
+const repositoryValidationEndpoint = "/validate-repository"
 
 var (
 	cert        tls.Certificate
@@ -67,10 +61,6 @@ var tracer = otel.Tracer("repository-webhook")
 
 // WebhookConfig defines the configuration for the Repository validation webhook
 type WebhookConfig struct {
-	Type                 WebhookType
-	ServiceName          string // only used if Type == WebhookTypeService
-	ServiceNamespace     string // only used if Type == WebhookTypeService
-	Host                 string // only used if Type == WebhookTypeUrl
 	Port                 int32
 	RepositoryPath       string
 	RepoServiceName      string
@@ -84,21 +74,6 @@ type WebhookConfig struct {
 // newWebhookConfig creates a new WebhookConfig object filled with values read from environment variables
 func newWebhookConfig(ctx context.Context) *WebhookConfig {
 	var cfg WebhookConfig
-	// NOTE: CERT_NAMESPACE is supported for backward compatibility.
-	// TODO: We may consider using only WEBHOOK_SERVICE_NAMESPACE instead.
-	if hasEnv("CERT_NAMESPACE") ||
-		hasEnv("WEBHOOK_SERVICE_NAME") ||
-		hasEnv("WEBHOOK_SERVICE_NAMESPACE") ||
-		!hasEnv("WEBHOOK_HOST") {
-
-		cfg.Type = WebhookTypeService
-		cfg.ServiceName, cfg.ServiceNamespace = webhookServiceName(ctx)
-		cfg.Host = fmt.Sprintf("%s.%s.svc", cfg.ServiceName, cfg.ServiceNamespace)
-	} else {
-		cfg.Type = WebhookTypeUrl
-		cfg.Host = getEnv("WEBHOOK_HOST", "localhost")
-	}
-	// Always use the WebhookTypeService for repository webhook validation
 	cfg.RepositoryPath = repositoryValidationEndpoint
 	cfg.RepoServiceName, cfg.RepoServiceNamespace = webhookServiceName(ctx)
 	cfg.RepoHost = fmt.Sprintf("%s.%s.svc", cfg.RepoServiceName, cfg.RepoServiceNamespace)
@@ -183,17 +158,9 @@ func setupWebhooks(ctx context.Context, clientReader client.Reader) error {
 }
 
 func createCerts(cfg *WebhookConfig) ([]byte, error) {
-	klog.Infof("creating self-signing TLS cert and key for %q in directory %s", cfg.Host, cfg.CertStorageDir)
-	commonName := cfg.Host
+	klog.Infof("creating self-signing TLS cert and key for %q in directory %s", cfg.RepoHost, cfg.CertStorageDir)
+	commonName := cfg.RepoHost
 	dnsNames := []string{commonName}
-	if cfg.Type == WebhookTypeService {
-		dnsNames = append(dnsNames, cfg.ServiceName)
-		dnsNames = append(dnsNames, fmt.Sprintf("%s.%s", cfg.ServiceName, cfg.ServiceNamespace))
-		dnsNames = append(dnsNames, fmt.Sprintf("%s.%s.svc", cfg.ServiceName, cfg.ServiceNamespace))
-		dnsNames = append(dnsNames, fmt.Sprintf("%s.%s.svc.cluster.local", cfg.ServiceName, cfg.ServiceNamespace))
-	}
-
-	// DNS names for CA config - repository-validating-webhook
 	dnsNames = append(dnsNames, cfg.RepoServiceName)
 	dnsNames = append(dnsNames, fmt.Sprintf("%s.%s", cfg.RepoServiceName, cfg.RepoServiceNamespace))
 	dnsNames = append(dnsNames, fmt.Sprintf("%s.%s.svc", cfg.RepoServiceName, cfg.RepoServiceNamespace))
@@ -295,7 +262,7 @@ func WriteFile(filepath string, c []byte) error {
 
 func createValidatingWebhook(ctx context.Context, cfg *WebhookConfig, caCert []byte) error {
 
-	klog.Infof("Creating validating webhook for %s:%d", cfg.Host, cfg.Port)
+	klog.Infof("Creating validating webhook for %s:%d", cfg.RepoHost, cfg.Port)
 
 	kubeConfig := ctrl.GetConfigOrDie()
 	kubeClient, err := kubernetes.NewForConfig(kubeConfig)
@@ -502,11 +469,6 @@ func writeErr(errMsg string, w *http.ResponseWriter) {
 	}
 }
 
-func hasEnv(key string) bool {
-	_, found := os.LookupEnv(key)
-	return found
-}
-
 func getEnv(key string, defaultValue string) string {
 	value, found := os.LookupEnv(key)
 	if !found {

pkg/apiserver/webhooks_test.go

@@ -38,8 +38,7 @@ import (
 
 func TestCreateCerts(t *testing.T) {
 	webhookCfg := WebhookConfig{
-		Type:           WebhookTypeUrl,
-		Host:           "localhost",
+		RepoHost:       "localhost",
 		CertStorageDir: t.TempDir(),
 	}
 	defer func() {
@@ -73,8 +72,7 @@ func TestLoadCertificate(t *testing.T) {
 	//what do i need to test.
 	// first create dummy certs for testing
 	webhookCfg := WebhookConfig{
-		Type:           WebhookTypeUrl,
-		Host:           "localhost",
+		RepoHost:       "localhost",
 		CertStorageDir: t.TempDir(),
 	}
 	defer func() {
@@ -149,8 +147,7 @@ func TestWatchCertificatesInvalidDirectory(t *testing.T) {
 	}
 	// Set up the temp directory with dummy certificate files
 	webhookCfg := WebhookConfig{
-		Type:           WebhookTypeUrl,
-		Host:           "localhost",
+		RepoHost:       "localhost",
 		CertStorageDir: t.TempDir(),
 	}
 	defer func() {
@@ -189,8 +186,7 @@ func TestWatchCertificatesSuccessfulReload(t *testing.T) {
 	}
 	// Set up the temp directory with dummy certificate files
 	webhookCfg := WebhookConfig{
-		Type:           WebhookTypeUrl,
-		Host:           "localhost",
+		RepoHost:       "localhost",
 		CertStorageDir: t.TempDir(),
 	}
 	defer func() {
@@ -238,8 +234,7 @@ func TestWatchCertificatesInvalidCertReload(t *testing.T) {
 	}
 	// Set up the temp directory with dummy certificate files
 	webhookCfg := WebhookConfig{
-		Type:           WebhookTypeUrl,
-		Host:           "localhost",
+		RepoHost:       "localhost",
 		CertStorageDir: t.TempDir(),
 	}
 	defer func() {
@@ -286,8 +281,7 @@ func TestWatchCertificatesGracefulTermination(t *testing.T) {
 	}
 	// Set up the temp directory with dummy certificate files
 	webhookCfg := WebhookConfig{
-		Type:           WebhookTypeUrl,
-		Host:           "localhost",
+		RepoHost:       "localhost",
 		CertStorageDir: t.TempDir(),
 	}
 	defer func() {

pkg/registry/porch/packagerevision_test.go

@@ -443,10 +443,12 @@ func TestWatch(t *testing.T) {
 	_, mockEngine := setup(t)
 	mockWatcherManager := mockengine.NewMockWatcherManager(t)
 	mockEngine.On("ObjectCache").Return(mockWatcherManager).Maybe()
-
 	mockWatcherManager.On("WatchPackageRevisions", mock.Anything, mock.Anything, mock.Anything).Return(fmt.Errorf("error starting watch")).Maybe()
 
-	_, err := packagerevisions.Watch(context.TODO(), &internalversion.ListOptions{})
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	_, err := packagerevisions.Watch(ctx, &internalversion.ListOptions{})
 	assert.NoError(t, err)
 
 	//=========================================================================================