diff --git a/cmd/openshift-tests/openshift-tests.go b/cmd/openshift-tests/openshift-tests.go index 0846627e3a11..4f4dd33ba557 100644 --- a/cmd/openshift-tests/openshift-tests.go +++ b/cmd/openshift-tests/openshift-tests.go @@ -23,6 +23,7 @@ import ( "github.com/openshift/origin/pkg/cmd/openshift-tests/dev" "github.com/openshift/origin/pkg/cmd/openshift-tests/disruption" e2e_analysis "github.com/openshift/origin/pkg/cmd/openshift-tests/e2e-analysis" + extensionadmission "github.com/openshift/origin/pkg/cmd/openshift-tests/extension-admission" "github.com/openshift/origin/pkg/cmd/openshift-tests/images" "github.com/openshift/origin/pkg/cmd/openshift-tests/list" "github.com/openshift/origin/pkg/cmd/openshift-tests/monitor" @@ -112,6 +113,7 @@ func main() { timeline.NewTimelineCommand(ioStreams), run_disruption.NewRunInClusterDisruptionMonitorCommand(ioStreams), collectdiskcertificates.NewRunCollectDiskCertificatesCommand(ioStreams), + extensionadmission.NewExtensionAdmissionCommand(ioStreams), versioncmd.NewVersionCommand(ioStreams), ) diff --git a/pkg/apis/testextension/v1/doc.go b/pkg/apis/testextension/v1/doc.go new file mode 100644 index 000000000000..c4a2b8d244a1 --- /dev/null +++ b/pkg/apis/testextension/v1/doc.go @@ -0,0 +1,5 @@ +// +k8s:deepcopy-gen=package + +// Package v1 contains API Schema definitions for the testextension v1 API group +// +groupName=testextension.redhat.io +package v1 diff --git a/pkg/apis/testextension/v1/register.go b/pkg/apis/testextension/v1/register.go new file mode 100644 index 000000000000..491ac41de7e3 --- /dev/null +++ b/pkg/apis/testextension/v1/register.go @@ -0,0 +1,14 @@ +package v1 + +import ( + "k8s.io/apimachinery/pkg/runtime/schema" +) + +const ( + GroupName = "testextension.redhat.io" + Version = "v1" +) + +var ( + SchemeGroupVersion = schema.GroupVersion{Group: GroupName, Version: Version} +) diff --git a/pkg/apis/testextension/v1/types.go b/pkg/apis/testextension/v1/types.go new file mode 100644 index 000000000000..6516bdce16fb --- /dev/null +++ b/pkg/apis/testextension/v1/types.go @@ -0,0 +1,44 @@ +package v1 + +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// +genclient +// +genclient:nonNamespaced +// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object + +// TestExtensionAdmission controls which ImageStreams are permitted to provide extension test binaries +type TestExtensionAdmission struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` + + Spec TestExtensionAdmissionSpec `json:"spec"` + Status TestExtensionAdmissionStatus `json:"status,omitempty"` +} + +// TestExtensionAdmissionSpec defines the desired state of TestExtensionAdmission +type TestExtensionAdmissionSpec struct { + // Permit is a list of permitted ImageStream patterns in format "namespace/imagestream". + // Supports wildcards like "openshift/*", "*/*", or specific "namespace/stream" + // +kubebuilder:validation:Required + // +kubebuilder:validation:MinItems=1 + Permit []string `json:"permit"` +} + +// TestExtensionAdmissionStatus defines the observed state of TestExtensionAdmission +type TestExtensionAdmissionStatus struct { + // Conditions represent the latest available observations of an object's state + // +optional + Conditions []metav1.Condition `json:"conditions,omitempty"` +} + +// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object + +// TestExtensionAdmissionList contains a list of TestExtensionAdmission +type TestExtensionAdmissionList struct { + metav1.TypeMeta `json:",inline"` + metav1.ListMeta `json:"metadata,omitempty"` + + Items []TestExtensionAdmission `json:"items"` +} diff --git a/pkg/cmd/openshift-tests/extension-admission/extension_admission_command.go b/pkg/cmd/openshift-tests/extension-admission/extension_admission_command.go new file mode 100644 index 000000000000..f1f2828cc3f6 --- /dev/null +++ b/pkg/cmd/openshift-tests/extension-admission/extension_admission_command.go @@ -0,0 +1,217 @@ +package extensionadmission + +import ( + _ "embed" + "fmt" + "os" + "os/exec" + "path/filepath" + "time" + + "github.com/spf13/cobra" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/cli-runtime/pkg/genericclioptions" + "k8s.io/kubectl/pkg/util/templates" + "sigs.k8s.io/yaml" + + testextensionv1 "github.com/openshift/origin/pkg/apis/testextension/v1" + "github.com/openshift/origin/pkg/cmd" + exutil "github.com/openshift/origin/test/extended/util" +) + +//go:embed testextensionadmission-crd.yaml +var crdYAML []byte + +func NewExtensionAdmissionCommand(ioStreams genericclioptions.IOStreams) *cobra.Command { + o := &extensionAdmissionOptions{ + ioStreams: ioStreams, + } + + command := &cobra.Command{ + Use: "extension-admission", + Short: "Manage TestExtensionAdmission resources", + Long: templates.LongDesc(` + Manage TestExtensionAdmission resources for controlling which ImageStreams + are permitted to provide extension test binaries. + + TestExtensionAdmission acts as an admission controller to determine which + ImageStreams are permitted to provide test binaries outside the main + OpenShift release payload. + + To list or delete TestExtensionAdmission resources, use standard kubectl/oc commands: + oc get testextensionadmissions + oc delete testextensionadmission + `), + PersistentPreRun: cmd.NoPrintVersion, + } + + command.AddCommand( + newInstallCRDCommand(o), + newCreateCommand(o), + ) + + return command +} + +type extensionAdmissionOptions struct { + ioStreams genericclioptions.IOStreams +} + +func newInstallCRDCommand(o *extensionAdmissionOptions) *cobra.Command { + command := &cobra.Command{ + Use: "install-crd", + Short: "Install the TestExtensionAdmission CRD", + Long: templates.LongDesc(` + Install the TestExtensionAdmission CustomResourceDefinition to the cluster. + + This CRD must be installed before creating TestExtensionAdmission instances. + `), + RunE: func(cmd *cobra.Command, args []string) error { + return o.installCRD() + }, + } + + return command +} + +func newCreateCommand(o *extensionAdmissionOptions) *cobra.Command { + createOpts := &createOptions{ + extensionAdmissionOptions: o, + } + + command := &cobra.Command{ + Use: "create NAME --permit=PATTERN [--permit=PATTERN...]", + Short: "Create a TestExtensionAdmission resource", + Long: templates.LongDesc(` + Create a TestExtensionAdmission resource with the specified permit patterns. + + Permit patterns are in the format "namespace/imagestream" and support wildcards: + - "openshift/*" - All ImageStreams in the openshift namespace + - "test-extensions/*" - All ImageStreams in test-extensions namespace + - "my-ns/my-stream" - Specific ImageStream + - "*/*" - All ImageStreams in all namespaces (use with caution) + + Example: + openshift-tests extension-admission create my-admission \ + --permit=openshift/* \ + --permit=test-extensions/* + `), + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + createOpts.name = args[0] + return createOpts.create() + }, + } + + command.Flags().StringSliceVar(&createOpts.permits, "permit", nil, "Permit pattern(s) (can be specified multiple times)") + command.MarkFlagRequired("permit") + + return command +} + +type createOptions struct { + *extensionAdmissionOptions + name string + permits []string +} + +func (o *createOptions) create() error { + if len(o.permits) == 0 { + return fmt.Errorf("at least one --permit pattern is required") + } + + // Create the TestExtensionAdmission object + admission := &testextensionv1.TestExtensionAdmission{ + TypeMeta: metav1.TypeMeta{ + APIVersion: testextensionv1.SchemeGroupVersion.String(), + Kind: "TestExtensionAdmission", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: o.name, + }, + Spec: testextensionv1.TestExtensionAdmissionSpec{ + Permit: o.permits, + }, + } + + // Convert to YAML + yamlBytes, err := yaml.Marshal(admission) + if err != nil { + return fmt.Errorf("failed to marshal TestExtensionAdmission to YAML: %w", err) + } + + // Apply using kubectl/oc + artifactName := fmt.Sprintf("testextensionadmission-%s.yaml", o.name) + if err := o.applyYAML(yamlBytes, artifactName); err != nil { + return fmt.Errorf("failed to apply TestExtensionAdmission: %w", err) + } + + fmt.Fprintf(o.ioStreams.Out, "TestExtensionAdmission %q created successfully\n", o.name) + return nil +} + +func (o *extensionAdmissionOptions) installCRD() error { + if err := o.applyYAML(crdYAML, "testextensionadmission-crd.yaml"); err != nil { + return fmt.Errorf("failed to install CRD: %w", err) + } + + fmt.Fprintln(o.ioStreams.Out, "TestExtensionAdmission CRD installed successfully") + return nil +} + +// saveToArtifactDir saves the YAML content to ARTIFACT_DIR if the environment variable is set. +// This helps with debugging by preserving the applied manifests. +func saveToArtifactDir(yamlBytes []byte, basename string) { + artifactDir := os.Getenv("ARTIFACT_DIR") + if artifactDir == "" { + return + } + + // Create a timestamped filename to avoid collisions + timestamp := time.Now().UTC().Format("20060102-150405") + filename := fmt.Sprintf("%s-%s", timestamp, basename) + artifactPath := filepath.Join(artifactDir, filename) + + if err := os.WriteFile(artifactPath, yamlBytes, 0644); err != nil { + // Don't fail the operation, just log the error + fmt.Fprintf(os.Stderr, "Warning: Failed to save artifact to %s: %v\n", artifactPath, err) + return + } + + fmt.Fprintf(os.Stderr, "Saved artifact to %s\n", artifactPath) +} + +func (o *extensionAdmissionOptions) applyYAML(yamlBytes []byte, artifactName string) error { + // Write YAML to a temporary file + tmpFile, err := os.CreateTemp("", "testextensionadmission-*.yaml") + if err != nil { + return fmt.Errorf("failed to create temp file: %w", err) + } + defer os.Remove(tmpFile.Name()) + + if _, err := tmpFile.Write(yamlBytes); err != nil { + tmpFile.Close() + return fmt.Errorf("failed to write YAML to temp file: %w", err) + } + tmpFile.Close() + + // Use oc apply via exec.Command + ocPath := "oc" + kubeconfig := exutil.KubeConfigPath() + + var cmd *exec.Cmd + if kubeconfig != "" { + cmd = exec.Command(ocPath, "--kubeconfig="+kubeconfig, "apply", "-f", tmpFile.Name()) + } else { + cmd = exec.Command(ocPath, "apply", "-f", tmpFile.Name()) + } + + output, err := cmd.CombinedOutput() + if err != nil { + return fmt.Errorf("oc apply failed: %w\nOutput: %s", err, string(output)) + } + + fmt.Fprintf(o.ioStreams.Out, "%s", string(output)) + saveToArtifactDir(yamlBytes, artifactName) + return nil +} diff --git a/pkg/cmd/openshift-tests/extension-admission/testextensionadmission-crd.yaml b/pkg/cmd/openshift-tests/extension-admission/testextensionadmission-crd.yaml new file mode 100644 index 000000000000..441b6a94febd --- /dev/null +++ b/pkg/cmd/openshift-tests/extension-admission/testextensionadmission-crd.yaml @@ -0,0 +1,43 @@ +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: testextensionadmissions.testextension.redhat.io +spec: + group: testextension.redhat.io + names: + kind: TestExtensionAdmission + listKind: TestExtensionAdmissionList + plural: testextensionadmissions + singular: testextensionadmission + shortNames: + - tea + scope: Cluster + versions: + - name: v1 + served: true + storage: true + schema: + openAPIV3Schema: + type: object + description: TestExtensionAdmission controls which ImageStreams are permitted to provide extension test binaries + properties: + spec: + type: object + description: Specification of permitted ImageStreams + properties: + permit: + type: array + description: List of permitted ImageStream patterns in format "namespace/imagestream". Each segment must be either "*" (wildcard) or a valid name (no embedded wildcards). Examples - "openshift/*", "*/*", "namespace/stream" + minItems: 1 + items: + type: string + pattern: '^(\*|[a-zA-Z0-9]([-_a-zA-Z0-9]*[a-zA-Z0-9])?)/(\*|[a-zA-Z0-9]([-_a-zA-Z0-9]*[a-zA-Z0-9])?)$' + required: + - permit + status: + type: object + description: Status of the TestExtensionAdmission + x-kubernetes-preserve-unknown-fields: true + subresources: + status: {} diff --git a/pkg/cmd/openshift-tests/images/images_command.go b/pkg/cmd/openshift-tests/images/images_command.go index 9661d31d5017..ae8f86137aea 100644 --- a/pkg/cmd/openshift-tests/images/images_command.go +++ b/pkg/cmd/openshift-tests/images/images_command.go @@ -122,7 +122,7 @@ func createImageMirrorForInternalImages(prefix string, ref reference.DockerImage // Extract all test binaries extractionContext, extractionContextCancel := context.WithTimeout(context.Background(), 30*time.Minute) defer extractionContextCancel() - cleanUpFn, externalBinaries, err := extensions.ExtractAllTestBinaries(extractionContext, 10) + cleanUpFn, externalBinaries, _, err := extensions.ExtractAllTestBinaries(extractionContext, 10) if err != nil { return nil, err } diff --git a/pkg/cmd/openshift-tests/list/extensions.go b/pkg/cmd/openshift-tests/list/extensions.go index f3a5b9e38c97..3675c76cbc61 100644 --- a/pkg/cmd/openshift-tests/list/extensions.go +++ b/pkg/cmd/openshift-tests/list/extensions.go @@ -47,7 +47,7 @@ func NewListExtensionsCommand(streams genericclioptions.IOStreams) *cobra.Comman } // Extract all test binaries from the release payload - cleanup, binaries, err := extensions.ExtractAllTestBinaries(ctx, 10) + cleanup, binaries, _, err := extensions.ExtractAllTestBinaries(ctx, 10) if err != nil { return fmt.Errorf("failed to extract test binaries: %w", err) } diff --git a/pkg/test/extensions/binary.go b/pkg/test/extensions/binary.go index b5e5469e8604..ae0ac51eb6cb 100644 --- a/pkg/test/extensions/binary.go +++ b/pkg/test/extensions/binary.go @@ -27,6 +27,8 @@ import ( "k8s.io/klog/v2" k8simage "k8s.io/kubernetes/test/utils/image" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "github.com/openshift/origin/pkg/clioptions/clusterdiscovery" "github.com/openshift/origin/pkg/clioptions/imagesetup" "github.com/openshift/origin/pkg/clioptions/upgradeoptions" @@ -149,6 +151,26 @@ type TestBinary struct { info *Extension } +// UnpermittedExtension describes a discovered non-payload extension that is not permitted by any TestExtensionAdmission. +// Used to generate a synthetic skip test. +type UnpermittedExtension struct { + Namespace string + ImageStream string + Tag string + Component string +} + +// nonPayloadSource describes a discovered ImageStreamTag that advertises an extension binary (permitted or not). +type nonPayloadSource struct { + Namespace string + ImageStream string + Tag string + ImageRef string + BinaryPath string + Component string + BinaryArgs string +} + // ImageSet maps a Kubernetes image ID to its corresponding configuration. // It represents a collection of container images with their registry, name, and version. type ImageSet map[k8simage.ImageID]k8simage.Config @@ -466,8 +488,9 @@ func (b *TestBinary) ListImages(ctx context.Context) (ImageSet, error) { } // ExtractAllTestBinaries determines the optimal release payload to use, and extracts all the external -// test binaries from it, and returns a slice of them. -func ExtractAllTestBinaries(ctx context.Context, parallelism int) (func(), TestBinaries, error) { +// test binaries from it (payload + permitted non-payload), and returns cleanup, binaries, and any +// unpermitted non-payload extensions for synthetic skip tests. +func ExtractAllTestBinaries(ctx context.Context, parallelism int) (func(), TestBinaries, []UnpermittedExtension, error) { if len(os.Getenv("OPENSHIFT_SKIP_EXTERNAL_TESTS")) > 0 { logrus.Warning("Using built-in tests only due to OPENSHIFT_SKIP_EXTERNAL_TESTS being set") var internalBinaries []*TestBinary @@ -477,11 +500,11 @@ func ExtractAllTestBinaries(ctx context.Context, parallelism int) (func(), TestB } } - return func() {}, internalBinaries, nil + return func() {}, internalBinaries, nil, nil } if parallelism < 1 { - return nil, nil, errors.New("parallelism must be greater than zero") + return nil, nil, nil, errors.New("parallelism must be greater than zero") } // Filter extension binaries based on environment variables @@ -489,12 +512,12 @@ func ExtractAllTestBinaries(ctx context.Context, parallelism int) (func(), TestB releaseImage, err := DetermineReleasePayloadImage() if err != nil { - return nil, nil, errors.WithMessage(err, "couldn't determine release image") + return nil, nil, nil, errors.WithMessage(err, "couldn't determine release image") } tmpDir, err := os.MkdirTemp("", "external-binary") if err != nil { - return nil, nil, fmt.Errorf("failed to create temporary directory: %w", err) + return nil, nil, nil, fmt.Errorf("failed to create temporary directory: %w", err) } defer os.RemoveAll(tmpDir) @@ -502,12 +525,22 @@ func ExtractAllTestBinaries(ctx context.Context, parallelism int) (func(), TestB oc := exutil.NewCLIWithoutNamespace("default") registryAuthFilePath, err := DetermineRegistryAuthFilePath(tmpDir, oc) if err != nil { - return nil, nil, fmt.Errorf("failed to determine registry auth file path: %w", err) + return nil, nil, nil, fmt.Errorf("failed to determine registry auth file path: %w", err) } externalBinaryProvider, err := NewExternalBinaryProvider(releaseImage, registryAuthFilePath) if err != nil { - return nil, nil, errors.WithMessage(err, "could not create external binary provider") + return nil, nil, nil, errors.WithMessage(err, "could not create external binary provider") + } + + permitPatterns, err := DiscoverNonPayloadBinaryAdmission(ctx, oc.AdminConfig()) + if err != nil { + logrus.Warnf("Skipping non-payload extension discovery (admission check failed): %v", err) + permitPatterns = nil + } + permittedNonPayload, unpermittedNonPayload, err := discoverNonPayloadExtensions(ctx, oc, permitPatterns) + if err != nil { + logrus.Warnf("Non-payload extension discovery failed: %v", err) } var ( @@ -577,10 +610,19 @@ func ExtractAllTestBinaries(ctx context.Context, parallelism int) (func(), TestB } if len(errs) > 0 { externalBinaryProvider.Cleanup() - return nil, nil, fmt.Errorf("encountered errors while extracting binaries: %s", strings.Join(errs, ";")) + return nil, nil, nil, fmt.Errorf("encountered errors while extracting binaries: %s", strings.Join(errs, ";")) } - return externalBinaryProvider.Cleanup, binaries, nil + for _, src := range permittedNonPayload { + tb, err := externalBinaryProvider.ExtractBinaryFromImage(src.ImageRef, src.BinaryPath, src.imageTag()) + if err != nil { + logrus.Warnf("Failed to extract non-payload extension from %s: %v", src.imageTag(), err) + continue + } + binaries = append(binaries, tb) + } + + return externalBinaryProvider.Cleanup, binaries, unpermittedNonPayload, nil } type TestBinaries []*TestBinary @@ -904,3 +946,92 @@ func (b *TestBinary) filterToApplicableEnvironmentFlags(envFlags EnvironmentFlag return filtered } + +func (s nonPayloadSource) imageTag() string { + return fmt.Sprintf("%s/%s:%s", s.Namespace, s.ImageStream, s.Tag) +} + +// discoverNonPayloadExtensions lists ImageStreamTags with ComponentAnnotation in all namespaces, +// parses binary annotation and image ref, and splits into permitted vs unpermitted by the admission result. +func discoverNonPayloadExtensions(ctx context.Context, oc *exutil.CLI, permitPatterns []PermitPattern) (permitted []nonPayloadSource, unpermitted []UnpermittedExtension, err error) { + if len(permitPatterns) == 0 { + return nil, nil, nil + } + imageClient := oc.AdminImageClient().ImageV1() + // Search all namespaces + list, err := imageClient.ImageStreamTags("").List(ctx, metav1.ListOptions{}) + if err != nil { + return nil, nil, fmt.Errorf("failed to list ImageStreamTags in all namespaces: %v", err) + } + for i := range list.Items { + ist := &list.Items[i] + component := ist.Annotations[ComponentAnnotation] + if component == "" { + continue + } + binAnnot := ist.Annotations[BinaryAnnotation] + if binAnnot == "" { + continue + } + binPath, binArgs := parseBinaryAnnotation(binAnnot) + if binPath == "" { + continue + } + imageRef := ist.Image.DockerImageReference + namespace := ist.Namespace + if imageRef == "" { + logrus.Warnf("ImageStreamTag %s/%s has no image reference", namespace, ist.Name) + continue + } + streamName, tagName := splitImageStreamTagName(ist.Name) + if streamName == "" || tagName == "" { + continue + } + source := nonPayloadSource{ + Namespace: namespace, + ImageStream: streamName, + Tag: tagName, + ImageRef: imageRef, + BinaryPath: binPath, + Component: component, + BinaryArgs: binArgs, + } + if MatchesAnyPermit(namespace, streamName, permitPatterns) { + permitted = append(permitted, source) + } else { + unpermitted = append(unpermitted, UnpermittedExtension{ + Namespace: namespace, + ImageStream: streamName, + Tag: tagName, + Component: component, + }) + } + } + + return permitted, unpermitted, nil +} + +func parseBinaryAnnotation(v string) (path, args string) { + v = strings.TrimSpace(v) + idx := strings.Index(v, " ") + if idx < 0 { + return v, "" + } + return strings.TrimSpace(v[:idx]), strings.TrimSpace(v[idx+1:]) +} + +func splitImageStreamTagName(name string) (stream, tag string) { + idx := strings.LastIndex(name, ":") + if idx < 0 { + return "", "" + } + return name[:idx], name[idx+1:] +} + +// truncateLine truncates a string to maxLen characters, adding "..." if truncated. +func truncateLine(s string, maxLen int) string { + if len(s) <= maxLen { + return s + } + return s[:maxLen] + "..." +} diff --git a/pkg/test/extensions/non_payload_binary_admission.go b/pkg/test/extensions/non_payload_binary_admission.go new file mode 100644 index 000000000000..07deafa3decf --- /dev/null +++ b/pkg/test/extensions/non_payload_binary_admission.go @@ -0,0 +1,122 @@ +package extensions + +import ( + "context" + "strings" + + "github.com/sirupsen/logrus" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/discovery" + "k8s.io/client-go/dynamic" + "k8s.io/client-go/rest" + + testextensionv1 "github.com/openshift/origin/pkg/apis/testextension/v1" +) + +const ( + // ComponentAnnotation is the annotation on ImageStreamTags that advertise a non-payload extension. + // Value format: product-name-component-name. + ComponentAnnotation = "testextension.redhat.io/component" + // BinaryAnnotation is the annotation on ImageStreamTags that identifies the binary path + // and optional args. Value format: .gz [--argument=value] + BinaryAnnotation = "testextension.redhat.io/binary" +) + +// PermitPattern represents one entry from TestExtensionAdmission spec.permit. +// Namespace and ImageStream may be "*" to match any. +// ImageStream is the ImageStream resource name only (tag is irrelevant for matching). +type PermitPattern struct { + Namespace string + ImageStream string +} + +// ParsePermitPattern parses a permit string "namespace/imagestream" into a PermitPattern. +// Supports "*" for namespace and/or imagestream (e.g. "openshift/*", "*/*", "ns/stream"). +func ParsePermitPattern(permit string) (PermitPattern, bool) { + parts := strings.SplitN(permit, "/", 2) + if len(parts) != 2 { + return PermitPattern{}, false + } + ns := strings.TrimSpace(parts[0]) + is := strings.TrimSpace(parts[1]) + if ns == "" || is == "" { + return PermitPattern{}, false + } + return PermitPattern{Namespace: ns, ImageStream: is}, true +} + +// MatchesAnyPermit returns true if (namespace, imagestream) matches any of the patterns. +func MatchesAnyPermit(namespace, imagestream string, patterns []PermitPattern) bool { + for _, p := range patterns { + nsMatch := p.Namespace == "*" || p.Namespace == namespace + isMatch := p.ImageStream == "*" || p.ImageStream == imagestream + if nsMatch && isMatch { + return true + } + } + return false +} + +// DiscoverNonPayloadBinaryAdmission checks for the TestExtensionAdmission CRD, lists instances, and builds the permit set +func DiscoverNonPayloadBinaryAdmission(ctx context.Context, config *rest.Config) ([]PermitPattern, error) { + discoveryClient, err := discovery.NewDiscoveryClientForConfig(config) + if err != nil { + return nil, err + } + + gv := testextensionv1.SchemeGroupVersion + _, err = discoveryClient.ServerResourcesForGroupVersion(gv.String()) + if err != nil { + if isNotFound(err) { + logrus.Infof("TestExtensionAdmission CRD not found; will discover extension ImageStreamTags cluster-wide and report all as unpermitted") + return nil, nil + } + return nil, err + } + + dynamicClient, err := dynamic.NewForConfig(config) + if err != nil { + return nil, err + } + + gvr := schema.GroupVersionResource{ + Group: testextensionv1.GroupName, + Version: testextensionv1.Version, + Resource: "testextensionadmissions", + } + list, err := dynamicClient.Resource(gvr).List(ctx, metav1.ListOptions{}) + if err != nil { + if isNotFound(err) { + return nil, nil + } + return nil, err + } + + var patterns []PermitPattern + for _, item := range list.Items { + var admission testextensionv1.TestExtensionAdmission + err := runtime.DefaultUnstructuredConverter.FromUnstructured(item.Object, &admission) + if err != nil { + logrus.Warnf("Failed to convert TestExtensionAdmission: %v", err) + continue + } + + for _, permitStr := range admission.Spec.Permit { + pat, ok := ParsePermitPattern(permitStr) + if !ok { + logrus.Warnf("Invalid permit pattern in TestExtensionAdmission %q: %q", admission.Name, permitStr) + continue + } + patterns = append(patterns, pat) + } + } + + return patterns, nil +} + +func isNotFound(err error) bool { + return apierrors.IsNotFound(err) +} diff --git a/pkg/test/extensions/provider.go b/pkg/test/extensions/provider.go index 1e6abc894277..18f7e99c05b1 100644 --- a/pkg/test/extensions/provider.go +++ b/pkg/test/extensions/provider.go @@ -29,8 +29,7 @@ type ExternalBinaryProvider struct { imageStream *imagev1.ImageStream } -func NewExternalBinaryProvider(releaseImage, registryAuthfilePath string) (*ExternalBinaryProvider, - error) { +func NewExternalBinaryProvider(releaseImage, registryAuthfilePath string) (*ExternalBinaryProvider, error) { oc := util.NewCLIWithoutNamespace("default") // Use a fixed cache or tmp directory for storing binaries @@ -87,6 +86,77 @@ func (provider *ExternalBinaryProvider) Cleanup() { provider.binPath = "" } +// extractBinary handles the common extraction logic with file locking and caching. +// It extracts binaryPath from imageRef into targetDir, ungzips, makes executable, and validates architecture. +func (provider *ExternalBinaryProvider) extractBinary(imageRef, binaryPath, targetDir, imageTag string) (extractedBinary string, extractDuration time.Duration, err error) { + // Define the final path for the binary (without .gz extension) + finalBinPath := filepath.Join(targetDir, strings.TrimSuffix(filepath.Base(binaryPath), ".gz")) + + // Acquire a file lock to prevent concurrent extraction of the same binary. + // This is necessary when multiple openshift-tests processes share the same cache directory. + lockPath := finalBinPath + ".lock" + lockFile, err := os.OpenFile(lockPath, os.O_CREATE|os.O_RDWR, 0644) + if err != nil { + return "", 0, fmt.Errorf("failed to create lock file %q: %w", lockPath, err) + } + defer lockFile.Close() + + if err := syscall.Flock(int(lockFile.Fd()), syscall.LOCK_EX); err != nil { + return "", 0, fmt.Errorf("failed to acquire lock on %q: %w", lockPath, err) + } + defer syscall.Flock(int(lockFile.Fd()), syscall.LOCK_UN) + + // Check if the binary already exists in cache + if _, err := os.Stat(finalBinPath); err == nil { + // Revalidate architecture compatibility before returning cached binary + if err := checkCompatibleArchitecture(finalBinPath); err != nil { + logrus.Warnf("Cached binary %s for %s is incompatible with current architecture, removing: %v", finalBinPath, imageTag, err) + if removeErr := os.Remove(finalBinPath); removeErr != nil { + logrus.Warnf("Failed to remove incompatible cached binary %s: %v", finalBinPath, removeErr) + } + // Continue with normal extraction flow + } else { + logrus.Infof("Using existing binary %s for %s", finalBinPath, imageTag) + return finalBinPath, 0, nil + } + } + + // Start the extraction process + startTime := time.Now() + if err := runImageExtract(imageRef, binaryPath, targetDir, provider.registryAuthFilePath); err != nil { + return "", 0, fmt.Errorf("failed extracting %q from %q: %w", binaryPath, imageRef, err) + } + extractDuration = time.Since(startTime) + + extractedBinary = filepath.Join(targetDir, filepath.Base(binaryPath)) + + // Verify that the extracted binary exists; "oc extract image" doesn't error when the path doesn't exist + if _, err := os.Stat(extractedBinary); err != nil { + if os.IsNotExist(err) { + return "", 0, fmt.Errorf("extracted binary at path %q does not exist in image %q", extractedBinary, imageRef) + } + return "", 0, fmt.Errorf("failed to stat extracted binary %q: %w", extractedBinary, err) + } + + // Support gzipped external binaries (handle decompression) + extractedBinary, err = ungzipFile(extractedBinary) + if err != nil { + return "", 0, fmt.Errorf("failed to decompress external binary %q: %w", binaryPath, err) + } + + // Make the extracted binary executable + if err := os.Chmod(extractedBinary, 0755); err != nil { + return "", 0, fmt.Errorf("failed making the binary %q executable: %w", extractedBinary, err) + } + + // Verify the binary is compatible with our architecture + if err := checkCompatibleArchitecture(extractedBinary); err != nil { + return "", 0, errors.WithMessage(err, "error checking binary architecture compatability") + } + + return extractedBinary, extractDuration, nil +} + // ExtractBinaryFromReleaseImage resolves the tag from the release image and extracts the binary, // checking if the binary is compatible with the current systems' architecture. It returns an error // if extraction fails or if the binary is incompatible. @@ -99,8 +169,7 @@ func (provider *ExternalBinaryProvider) ExtractBinaryFromReleaseImage(tag, binar return nil, fmt.Errorf("extraction path is not set, cleanup was already run") } - // Allow overriding image path to an already existing local path, mostly useful - // for development. + // Allow overriding image path to an already existing local path, mostly useful for development if override := binaryPathOverride(tag, binary); override != "" { logrus.WithFields(logrus.Fields{ "tag": tag, @@ -113,7 +182,7 @@ func (provider *ExternalBinaryProvider) ExtractBinaryFromReleaseImage(tag, binar }, nil } - // Resolve the image tag from the image stream. + // Resolve the image tag from the image stream image := "" for _, t := range provider.imageStream.Spec.Tags { if t.Name == tag { @@ -126,82 +195,74 @@ func (provider *ExternalBinaryProvider) ExtractBinaryFromReleaseImage(tag, binar return nil, fmt.Errorf("%s not found", tag) } - // Define the path for the binary - binPath := filepath.Join(provider.binPath, strings.TrimSuffix(filepath.Base(binary), ".gz")) - - // Acquire a file lock to prevent concurrent extraction of the same binary. - // This is necessary when multiple openshift-tests processes share the same cache directory. - lockPath := binPath + ".lock" - lockFile, err := os.OpenFile(lockPath, os.O_CREATE|os.O_RDWR, 0644) + // Extract the binary using common logic + extractedBinary, extractDuration, err := provider.extractBinary(image, binary, provider.binPath, tag) if err != nil { - return nil, fmt.Errorf("failed to create lock file %q: %w", lockPath, err) - } - defer lockFile.Close() - - if err := syscall.Flock(int(lockFile.Fd()), syscall.LOCK_EX); err != nil { - return nil, fmt.Errorf("failed to acquire lock on %q: %w", lockPath, err) + return nil, err } - defer syscall.Flock(int(lockFile.Fd()), syscall.LOCK_UN) - // Check if the binary already exists - if _, err := os.Stat(binPath); err == nil { - logrus.Infof("Using existing binary %s for tag %s", binPath, tag) - return &TestBinary{ - imageTag: tag, - binaryPath: binPath, - }, nil - } - - // Start the extraction process. - startTime := time.Now() - if err := runImageExtract(image, binary, provider.binPath, provider.registryAuthFilePath); err != nil { - return nil, fmt.Errorf("failed extracting %q from %q: %w", binary, image, err) + // Log extraction details (file size only if we actually extracted) + if extractDuration > 0 { + if fileInfo, statErr := os.Stat(extractedBinary); statErr == nil { + logrus.Infof("Extracted %s for tag %s from %s (disk size %v, extraction duration %v)", + binary, tag, image, fileInfo.Size(), extractDuration) + } } - extractDuration := time.Since(startTime) - extractedBinary := filepath.Join(provider.binPath, filepath.Base(binary)) + return &TestBinary{ + imageTag: tag, + binaryPath: extractedBinary, + }, nil +} - // Verify that the extracted binary exists; "oc extract image" doesn't error when the path doesn't exist, - // so we check that the extraction was successful via its existence - _, err = os.Stat(extractedBinary) - if err != nil { - if os.IsNotExist(err) { - return nil, fmt.Errorf("extracted binary at path %q does not exist. the src path %q doesn't exist in image %q. note the version of origin needs to match the version of the cluster under test", - extractedBinary, binary, image) - } - return nil, fmt.Errorf("failed to stat external binary to check for existence %q: %w", binary, err) +// ExtractBinaryFromImage extracts a binary from an arbitrary image (e.g. from a non-payload ImageStreamTag). +// imageRef is the pull spec; binaryPath is the path inside the image; imageTag is the identifier for the +// TestBinary (e.g. namespace/imagestream:tag for non-payload). Uses the same lock/cache/ungzip/arch check as ExtractBinaryFromReleaseImage. +func (provider *ExternalBinaryProvider) ExtractBinaryFromImage(imageRef, binaryPath, imageTag string) (*TestBinary, error) { + if provider.binPath == "" { + return nil, fmt.Errorf("extraction path is not set, cleanup was already run") } - // Support gzipped external binaries (handle decompression). - extractedBinary, err = ungzipFile(extractedBinary) - if err != nil { - return nil, fmt.Errorf("failed to decompress external binary %q: %w", binary, err) + // Allow overriding image path to an already existing local path, mostly useful for development + if override := binaryPathOverride(imageTag, binaryPath); override != "" { + logrus.WithFields(logrus.Fields{ + "tag": imageTag, + "binary": binaryPath, + "override": override, + }).Info("Found override for this extension") + return &TestBinary{ + imageTag: imageTag, + binaryPath: override, + }, nil } - // Make the extracted binary executable. - if err := os.Chmod(extractedBinary, 0755); err != nil { - return nil, fmt.Errorf("failed making the extracted binary %q executable: %w", extractedBinary, err) + // Create a subdirectory for non-payload binaries to keep them separate + nonPayloadDir := filepath.Join(provider.binPath, "non-payload", safeNameForDir(imageTag)) + if err := createBinPath(nonPayloadDir); err != nil { + return nil, errors.WithMessagef(err, "error creating cache path %s", nonPayloadDir) } - // Verify the binary actually exists - fileInfo, err := os.Stat(extractedBinary) + // Extract the binary using common logic + extractedBinary, extractDuration, err := provider.extractBinary(imageRef, binaryPath, nonPayloadDir, imageTag) if err != nil { - return nil, fmt.Errorf("failed stat on extracted binary %q: %w", extractedBinary, err) + return nil, err } - // Verify the binary is compatible with our architecture - if err := checkCompatibleArchitecture(extractedBinary); err != nil { - return nil, errors.WithMessage(err, "error checking binary architecture compatability") + // Log extraction details + if extractDuration > 0 { + logrus.Infof("Extracted %s for %s from %s in %v", binaryPath, imageTag, imageRef, extractDuration) } - logrus.Infof("Extracted %s for tag %s from %s (disk size %v, extraction duration %v)", - binary, tag, image, fileInfo.Size(), extractDuration) - return &TestBinary{ + imageTag: imageTag, binaryPath: extractedBinary, }, nil } +func safeNameForDir(s string) string { + return strings.NewReplacer("/", "_", ":", "_").Replace(s) +} + func cleanOldCacheFiles(dir string) { maxAge := 24 * 7 * time.Hour // 7 days logrus.Infof("Cleaning up older cached data...") @@ -229,7 +290,7 @@ func cleanOldCacheFiles(dir string) { } func binaryPathOverride(imageTag, binaryPath string) string { - safeEnvVar := strings.NewReplacer("/", "_", "-", "_", ".", "_") + safeEnvVar := strings.NewReplacer("/", "_", "-", "_", ".", "_", ":", "_") // Check for a specific override for this binary path, less common but allows supporting // images that have multiple test binaries. diff --git a/pkg/test/ginkgo/cmd_runsuite.go b/pkg/test/ginkgo/cmd_runsuite.go index fae330a9abc9..ab76a4a8b6c0 100644 --- a/pkg/test/ginkgo/cmd_runsuite.go +++ b/pkg/test/ginkgo/cmd_runsuite.go @@ -159,6 +159,38 @@ func (o *GinkgoRunSuiteOptions) SetIOStreams(streams genericclioptions.IOStreams o.IOStreams = streams } +// createUnpermittedExtensionTests creates JUnit test cases to report unpermitted extensions. +// Returns a flake (pass + fail) to make it visible without failing the job. +func createUnpermittedExtensionTests(unpermitted []extensions.UnpermittedExtension) []*junitapi.JUnitTestCase { + const testName = "[openshift-tests] extension binary not permitted" + testCases := []*junitapi.JUnitTestCase{ + { + Name: testName, + }, + } + + // If unpermitted extensions found, also create a failure case to make it a flake + if len(unpermitted) > 0 { + logrus.Warnf("Found %d unpermitted extension binary(ies)", len(unpermitted)) + + var msg strings.Builder + msg.WriteString("extension binary not permitted by TestExtensionAdmission:\n") + for _, u := range unpermitted { + fmt.Fprintf(&msg, "\n - %s/%s:%s (%s)\n", u.Namespace, u.ImageStream, u.Tag, u.Component) + logrus.Warnf(" Unpermitted: %s/%s:%s (%s)", u.Namespace, u.ImageStream, u.Tag, u.Component) + } + + testCases = append(testCases, &junitapi.JUnitTestCase{ + Name: testName, + FailureOutput: &junitapi.FailureOutput{ + Output: msg.String(), + }, + }) + } + + return testCases +} + func max(a, b int) int { if a > b { return a @@ -180,7 +212,7 @@ func (o *GinkgoRunSuiteOptions) Run(suite *TestSuite, clusterConfig *clusterdisc // Extract all test binaries extractionContext, extractionContextCancel := context.WithTimeout(context.Background(), 30*time.Minute) defer extractionContextCancel() - cleanUpFn, allBinaries, err := extensions.ExtractAllTestBinaries(extractionContext, defaultBinaryParallelism) + cleanUpFn, allBinaries, unpermitted, err := extensions.ExtractAllTestBinaries(extractionContext, defaultBinaryParallelism) if err != nil { return err } @@ -244,6 +276,8 @@ func (o *GinkgoRunSuiteOptions) Run(suite *TestSuite, clusterConfig *clusterdisc return fmt.Errorf("no tests to run") } + unpermittedTestCases := createUnpermittedExtensionTests(unpermitted) + k8sTestNames := map[string]bool{} for _, t := range specs { if strings.Contains(t.ExtensionTestSpec.Source, "hyperkube") { @@ -616,6 +650,7 @@ func (o *GinkgoRunSuiteOptions) Run(suite *TestSuite, clusterConfig *clusterdisc var syntheticTestResults []*junitapi.JUnitTestCase var syntheticFailure bool syntheticTestResults = append(syntheticTestResults, stableClusterTestResults...) + syntheticTestResults = append(syntheticTestResults, unpermittedTestCases...) timeSuffix := fmt.Sprintf("_%s", start.UTC().Format("20060102-150405")) diff --git a/pkg/testsuites/standard_suites.go b/pkg/testsuites/standard_suites.go index b4ecc9f89d9b..659f2d08d884 100644 --- a/pkg/testsuites/standard_suites.go +++ b/pkg/testsuites/standard_suites.go @@ -49,7 +49,7 @@ func AllTestSuites(ctx context.Context) ([]*ginkgo.TestSuite, error) { } // Extract all test binaries from the release payload - cleanup, binaries, err := extensions.ExtractAllTestBinaries(ctx, 10) + cleanup, binaries, _, err := extensions.ExtractAllTestBinaries(ctx, 10) if err != nil { return nil, fmt.Errorf("failed to extract test binaries: %w", err) }