address ai review

Author: mjudeikis

cli/pkg/kubectl/bind-apiservice/plugin/bind.go

@@ -117,7 +117,7 @@ func (b *BindAPIServiceOptions) Complete(args []string) error {
 // Validate validates the BindAPIServiceOptions are complete and usable.
 func (b *BindAPIServiceOptions) Validate() error {
 	if b.file == "" && b.Template == "" {
-		return errors.New("file, template-name or arguments are required")
+		return errors.New("file, template-name or --file are required")
 	}
 
 	if allowed := sets.NewString(b.Print.AllowedFormats()...); *b.Print.OutputFormat != "" && !allowed.Has(*b.Print.OutputFormat) {
@@ -133,7 +133,8 @@ func (b *BindAPIServiceOptions) Validate() error {
 			return errors.New("remote-kubeconfig or remote-kubeconfig-namespace and remote-kubeconfig-name are required")
 		}
 	}
-	if b.Name == "" {
+	// Name is required unless reading from file, where name will be read from the file.
+	if b.Name == "" && b.file == "" {
 		return errors.New("name is required")
 	}
 
@@ -149,11 +150,6 @@ func (b *BindAPIServiceOptions) Run(ctx context.Context) error {
 		return err
 	}
 
-	r, err := b.bindTemplate(ctx)
-	if err != nil {
-		return err
-	}
-
 	// Use the shared binder to create bindings
 	binderOpts := &BinderOptions{
 		IOStreams:                 b.Options.IOStreams,
@@ -164,12 +160,25 @@ func (b *BindAPIServiceOptions) Run(ctx context.Context) error {
 		RemoteKubeconfigNamespace: b.remoteKubeconfigNamespace,
 		RemoteKubeconfigName:      b.remoteKubeconfigName,
 		RemoteNamespace:           b.remoteNamespace,
+		File:                      b.file,
 	}
-
 	binder := NewBinder(config, binderOpts)
-	bindings, err := binder.BindFromResponse(ctx, r.response)
-	if err != nil {
-		return fmt.Errorf("failed to create bindings: %w", err)
+
+	var bindings []*kubebindv1alpha2.APIServiceBinding
+	if b.Template != "" {
+		r, err := b.bindTemplate(ctx)
+		if err != nil {
+			return err
+		}
+		bindings, err = binder.BindFromResponse(ctx, r.response)
+		if err != nil {
+			return fmt.Errorf("failed to create bindings: %w", err)
+		}
+	} else if b.file != "" {
+		bindings, err = binder.BindFromFile(ctx)
+		if err != nil {
+			return fmt.Errorf("failed to create bindings: %w", err)
+		}
 	}
 
 	fmt.Fprintln(b.Options.ErrOut)

cli/pkg/kubectl/bind-apiservice/plugin/binder.go

@@ -20,11 +20,14 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"io"
+	"os"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/cli-runtime/pkg/genericclioptions"
 	kubeclient "k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/rest"
+	"sigs.k8s.io/yaml"
 
 	"github.com/kube-bind/kube-bind/cli/pkg/kubectl/base"
 	kubebindv1alpha2 "github.com/kube-bind/kube-bind/sdk/apis/kubebind/v1alpha2"
@@ -40,6 +43,7 @@ type BinderOptions struct {
 	RemoteKubeconfigNamespace string
 	RemoteKubeconfigName      string
 	RemoteNamespace           string
+	File                      string
 	DryRun                    bool
 }
 
@@ -57,6 +61,95 @@ func NewBinder(config *rest.Config, opts *BinderOptions) *Binder {
 	}
 }
 
+// TODO: bindFromFile and bindFromResponse can likely share a lot of code. This slow is bit repetitive
+// but keeps the two paths separate for clarity. But it needs love.
+
+func (b *Binder) BindFromFile(ctx context.Context) ([]*kubebindv1alpha2.APIServiceBinding, error) {
+	// Ensure client side namespace exists
+	err := b.ensureClientSideNamespaceExists(ctx)
+	if err != nil {
+		return nil, fmt.Errorf("failed to ensure kube-bind namespace exists: %w", err)
+	}
+
+	remoteKubeconfig, _, _, err := b.getRemoteKubeconfig(ctx, "", "")
+	if err != nil {
+		return nil, err
+	}
+
+	// Copy kubeconfig into local cluster
+	remoteHost, remoteNamespace, err := base.ParseRemoteKubeconfig([]byte(remoteKubeconfig))
+	if err != nil {
+		return nil, err
+	}
+
+	kubeClient, err := kubeclient.NewForConfig(b.config)
+	if err != nil {
+		return nil, fmt.Errorf("failed to create kube client: %w", err)
+	}
+
+	secretName, err := base.FindRemoteKubeconfig(ctx, kubeClient, remoteNamespace, remoteHost)
+	if err != nil {
+		return nil, err
+	}
+
+	secret, created, err := base.EnsureKubeconfigSecret(ctx, string(remoteKubeconfig), secretName, kubeClient)
+	if err != nil {
+		return nil, err
+	}
+
+	if created {
+		fmt.Fprintf(b.opts.IOStreams.ErrOut, "Created secret %s/%s for host %s, namespace %s\n", "kube-bind", secret.Name, remoteHost, remoteNamespace)
+	} else {
+		fmt.Fprintf(b.opts.IOStreams.ErrOut, "Updated secret %s/%s for host %s, namespace %s\n", "kube-bind", secret.Name, remoteHost, remoteNamespace)
+	}
+
+	if b.opts.DryRun {
+		return nil, nil
+	}
+
+	// Get remote kubeconfig
+	remoteKubeconfig, remoteNamespaceActual, remoteConfig, err := b.getRemoteKubeconfig(ctx, secret.Namespace, secret.Name)
+	if err != nil {
+		return nil, fmt.Errorf("failed to get remote kubeconfig: %w", err)
+	}
+
+	data, err := b.getRequestManifest()
+	if err != nil {
+		return nil, fmt.Errorf("failed to get request manifest: %w", err)
+	}
+
+	request, err := b.unmarshalManifest(data)
+	if err != nil {
+		return nil, fmt.Errorf("failed to unmarshal request manifest: %w", err)
+	}
+
+	// Deploy konnector if needed
+	if err := b.deployKonnector(ctx); err != nil {
+		return nil, fmt.Errorf("failed to deploy konnector: %w", err)
+	}
+
+	// Create bindings for all requests
+	var bindings []*kubebindv1alpha2.APIServiceBinding
+	result, err := b.createServiceExportRequest(ctx, remoteConfig, remoteNamespaceActual, request)
+	if err != nil {
+		return nil, fmt.Errorf("failed to create service export request: %w", err)
+	}
+
+	secretName, err = b.createKubeconfigSecret(ctx, remoteConfig.Host, remoteNamespaceActual, remoteKubeconfig)
+	if err != nil {
+		return nil, fmt.Errorf("failed to create kubeconfig secret: %w", err)
+	}
+
+	results, err := b.createAPIServiceBindings(ctx, result, secretName)
+	if err != nil {
+		return nil, fmt.Errorf("failed to create API service bindings: %w", err)
+	}
+	bindings = append(bindings, results...)
+
+	return bindings, nil
+
+}
+
 // BindFromResponse processes a BindingResourceResponse and creates all necessary bindings
 func (b *Binder) BindFromResponse(ctx context.Context, response *kubebindv1alpha2.BindingResourceResponse) ([]*kubebindv1alpha2.APIServiceBinding, error) {
 	if response.Authentication.OAuth2CodeGrant == nil {
@@ -96,6 +189,10 @@ func (b *Binder) BindFromResponse(ctx context.Context, response *kubebindv1alpha
 		fmt.Fprintf(b.opts.IOStreams.ErrOut, "Updated secret %s/%s for host %s, namespace %s\n", "kube-bind", secret.Name, remoteHost, remoteNamespace)
 	}
 
+	if b.opts.DryRun {
+		return nil, nil
+	}
+
 	// Get remote kubeconfig
 	remoteKubeconfig, remoteNamespaceActual, remoteConfig, err := b.getRemoteKubeconfig(ctx, secret.Namespace, secret.Name)
 	if err != nil {
@@ -172,6 +269,7 @@ func (b *Binder) deployKonnector(ctx context.Context) error {
 		SkipKonnector:          b.opts.SkipKonnector,
 		KonnectorImageOverride: b.opts.KonnectorImageOverride,
 		DowngradeKonnector:     b.opts.DowngradeKonnector,
+		DryRun:                 b.opts.DryRun,
 	}
 	return tempOpts.deployKonnector(ctx, b.config)
 }
@@ -198,3 +296,33 @@ func (b *Binder) createAPIServiceBindings(ctx context.Context, request *kubebind
 	}
 	return tempOpts.createAPIServiceBindings(ctx, b.config, request, secretName)
 }
+
+func (b *Binder) getRequestManifest() ([]byte, error) {
+	if b.opts.File == "-" {
+		body, err := io.ReadAll(b.opts.IOStreams.In)
+		if err != nil {
+			return nil, fmt.Errorf("failed to read from stdin: %w", err)
+		}
+		return body, nil
+	}
+
+	body, err := os.ReadFile(b.opts.File)
+	if err != nil {
+		return nil, fmt.Errorf("failed to read file %s: %w", b.opts.File, err)
+	}
+	return body, nil
+}
+
+func (b *Binder) unmarshalManifest(bs []byte) (*kubebindv1alpha2.APIServiceExportRequest, error) {
+	var request kubebindv1alpha2.APIServiceExportRequest
+	if err := yaml.Unmarshal(bs, &request); err != nil {
+		return nil, fmt.Errorf("failed to unmarshal manifest: %w", err)
+	}
+	if request.APIVersion != kubebindv1alpha2.SchemeGroupVersion.String() {
+		return nil, fmt.Errorf("invalid apiVersion %q", request.APIVersion)
+	}
+	if request.Kind != "APIServiceExportRequest" {
+		return nil, fmt.Errorf("invalid kind %q", request.Kind)
+	}
+	return &request, nil
+}

cli/pkg/kubectl/bind-login/plugin/login.go

@@ -134,7 +134,7 @@ func (o *LoginOptions) Run(ctx context.Context, authURLCh chan<- string) error {
 	tokenCh := make(chan *TokenResponse, 1)
 	errCh := make(chan error, 1)
 
-	server, localCallbackURL, err := o.startCallbackServerWithRandomPort(tokenCh, errCh)
+	server, localCallbackURL, err := o.startCallbackServerWithRandomPort(sessionID, tokenCh, errCh)
 	if err != nil {
 		return fmt.Errorf("failed to start callback server: %w", err)
 	}
@@ -227,7 +227,11 @@ func (o *LoginOptions) Run(ctx context.Context, authURLCh chan<- string) error {
 	}
 
 	if o.ShowToken {
-		fmt.Fprintf(o.Streams.ErrOut, "\nStored token: %s\n", token.AccessToken[:20]+"...")
+		displayToken := token.AccessToken
+		if len(displayToken) > 20 {
+			displayToken = displayToken[:20] + "..."
+		}
+		fmt.Fprintf(o.Streams.ErrOut, "\nStored token: %s\n", displayToken)
 	}
 
 	configPath, _ := config.GetConfigPath()
@@ -304,21 +308,25 @@ func (o *LoginOptions) buildAuthURL(provider *kubebindv1alpha2.BindingProvider,
 	return u.String(), nil
 }
 
-func (o *LoginOptions) startCallbackServerWithRandomPort(tokenCh chan<- *TokenResponse, errCh chan<- error) (*http.Server, string, error) {
-	// Try to find an available port
+func (o *LoginOptions) startCallbackServerWithRandomPort(sessionID string, tokenCh chan<- *TokenResponse, errCh chan<- error) (*http.Server, string, error) {
+	expectedSessionID := sessionID
 	listener, err := net.Listen("tcp", "127.0.0.1:0")
 	if err != nil {
 		return nil, "", fmt.Errorf("failed to find available port: %w", err)
 	}
 
 	port := listener.Addr().(*net.TCPAddr).Port
-	listener.Close()
 
 	callbackURL := fmt.Sprintf("http://127.0.0.1:%d/callback", port)
 
 	// Setup HTTP handler
 	mux := http.NewServeMux()
 	mux.HandleFunc("/callback", func(w http.ResponseWriter, r *http.Request) {
+		receivedSessionID := r.URL.Query().Get("session_id")
+		if receivedSessionID != expectedSessionID {
+			http.Error(w, "Invalid session", http.StatusBadRequest)
+			return
+		}
 		token := &TokenResponse{
 			Error:        r.URL.Query().Get("error"),
 			ErrorMessage: r.URL.Query().Get("error_description"),

cli/pkg/kubectl/bind/plugin/bind.go

@@ -158,7 +158,11 @@ func (b *BindOptions) runWithCallback(ctx context.Context, _ chan<- string) erro
 	if err != nil {
 		return fmt.Errorf("failed to start callback server: %w", err)
 	}
-	defer callbackServer.Close()
+	defer func() {
+		shutdownCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+		defer cancel()
+		_ = callbackServer.Shutdown(shutdownCtx)
+	}()
 
 	// Build the UI URL with callback parameters
 	uiURL, err := b.buildUIURL(callbackPort, sessionID, b.Cluster)
@@ -222,7 +226,6 @@ func (b *BindOptions) runWithCallback(ctx context.Context, _ chan<- string) erro
 					fmt.Fprintf(b.Options.IOStreams.Out, "%s", request.Raw)
 				}
 			}
-			return nil
 		}
 
 		// Create bindings using the shared binder
@@ -236,6 +239,11 @@ func (b *BindOptions) runWithCallback(ctx context.Context, _ chan<- string) erro
 			return fmt.Errorf("failed to create APIServiceBindings: %w", err)
 		}
 
+		if b.DryRun {
+			fmt.Fprintf(b.Options.IOStreams.ErrOut, "\nDry-run mode: no APIServiceBindings were created.\n")
+			return nil
+		}
+
 		// Print the results
 		if len(bindings) > 0 {
 			fmt.Fprintf(b.Options.IOStreams.ErrOut, "Created %d APIServiceBinding(s):\n", len(bindings))