MGMT-15715: Allow handling multi-document YAML custom manifests (#5480)

* MGMT-15715: Allow handling multi-document YAML custom manifests Multi document yamls are not supported during installation. This change looks into the custom manifests files uploaded by the user, and splits multi document yaml files into several ones. * update invalid yaml content * better test description * rename other-metadata into some-other-manifest-source * update isValidYaml description * return error instead of bool in isYamlValid * refactor GetUserManifestSuffixes * fix unit tests * add documentation about accepted inputs
openshift · Sep 15, 2023 · 110fc6d · 110fc6d
1 parent 12ca0a4
commit 110fc6d
Show file tree

Hide file tree

Showing 5 changed files with 436 additions and 11 deletions.
diff --git a/docs/user-guide/install-customization.md b/docs/user-guide/install-customization.md
@@ -6,6 +6,9 @@ These APIs allows for adding arbitrary manifests to the set generated by `opensh
 A typical use case would be to create a MachineConfig which would make some persistent customization to a group of nodes.
 These will only take effect after the machine config operator is up and running so if a change is needed before or during the installation process, another API will be required.
 
+Formats json, yaml, and multi-document yaml are accepted as manifests.
+Multi-document yaml manifests are split with unique names to avoid any collisions with existing files when they are added to their respective folders (either "manifests" or "openshift").
+
 ### Create a cluster manifest
 
 Note: In environments where TLS is enabled (e.g. _api.openshift.com_), you will need to use the `https` scheme in URLs.

diff --git a/internal/ignition/ignition.go b/internal/ignition/ignition.go
@@ -26,6 +26,7 @@ import (
 	"github.com/coreos/vcontext/report"
 	"github.com/go-openapi/strfmt"
 	"github.com/go-openapi/swag"
+	"github.com/google/uuid"
 	bmh_v1alpha1 "github.com/metal3-io/baremetal-operator/apis/metal3.io/v1alpha1"
 	clusterPkg "github.com/openshift/assisted-service/internal/cluster"
 	"github.com/openshift/assisted-service/internal/common"
@@ -409,6 +410,12 @@ func (g *installerGenerator) Generate(ctx context.Context, installConfig []byte,
 		}
 	}
 
+	err = g.expandUserMultiDocYamls(ctx)
+	if err != nil {
+		log.WithError(err).Errorf("failed expand multi-document yaml for cluster '%s'", g.cluster.ID)
+		return err
+	}
+
 	err = g.applyManifestPatches(ctx)
 	if err != nil {
 		log.WithError(err).Errorf("failed to apply manifests' patches for cluster '%s'", g.cluster.ID)
@@ -679,6 +686,110 @@ func (g *installerGenerator) bootstrapInPlaceIgnitionsCreate(ctx context.Context
 	return nil
 }
 
+// expandUserMultiDocYamls finds if user uploaded multi document yaml files and
+// split them into several files
+func (g *installerGenerator) expandUserMultiDocYamls(ctx context.Context) error {
+	log := logutil.FromContext(ctx, g.log)
+
+	metadata, err := manifests.GetManifestMetadata(ctx, g.cluster.ID, g.s3Client)
+	if err != nil {
+		return errors.Wrapf(err, "Failed to retrieve manifest matadata")
+	}
+	userManifests, err := manifests.ResolveManifestNamesFromMetadata(
+		manifests.FilterMetadataOnManifestSource(metadata, constants.ManifestSourceUserSupplied),
+	)
+	if err != nil {
+		return errors.Wrapf(err, "Failed to resolve manifest names from metadata")
+	}
+
+	// pass a random token to expandMultiDocYaml in order to prevent name
+	// clashes when spliting one file into several ones
+	randomToken := uuid.NewString()[:7]
+
+	for _, manifest := range userManifests {
+		log.Debugf("Looking at expanding manifest file %s", manifest)
+
+		extension := filepath.Ext(manifest)
+		if !(extension == ".yaml" || extension == ".yml") {
+			continue
+		}
+
+		manifestPath := filepath.Join(g.workDir, manifest)
+		err := g.expandMultiDocYaml(ctx, manifestPath, randomToken)
+		if err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+// expandMultiDocYaml splits a multi document yaml file into several files
+// if the the file given in input contains only one document, the file is left untouched
+func (g *installerGenerator) expandMultiDocYaml(ctx context.Context, manifestPath string, uniqueToken string) error {
+	var err error
+
+	log := logutil.FromContext(ctx, g.log)
+
+	data, err := os.ReadFile(manifestPath)
+	if err != nil {
+		return errors.Wrapf(err, "Failed to read %s", manifestPath)
+	}
+
+	// read each yaml document contained in the file into a slice
+	manifestContentList := [][]byte{}
+	dec := yaml.NewDecoder(bytes.NewReader(data))
+	for {
+		var doc interface{}
+		err = dec.Decode(&doc)
+		if errors.Is(err, io.EOF) {
+			break
+		}
+		if err != nil {
+			return errors.Wrapf(err, "Failed to parse yaml document %s", manifestPath)
+		}
+
+		// skip empty documents
+		if doc == nil {
+			continue
+		}
+
+		manifestContent, marshalError := yaml.Marshal(doc)
+		if marshalError != nil {
+			return errors.Wrapf(err, "Failed to re-encode yaml file %s", manifestPath)
+		}
+		manifestContentList = append(manifestContentList, manifestContent)
+	}
+
+	if len(manifestContentList) <= 1 {
+		return nil
+	}
+
+	log.Infof("Expanding multi-document yaml file %s into %d files", manifestPath, len(manifestContentList))
+
+	// if the yaml file contains more than one document,
+	// split it into several files
+	for idx, content := range manifestContentList {
+		fileExt := filepath.Ext(manifestPath)
+		fileWithoutExt := strings.TrimSuffix(manifestPath, fileExt)
+		filename := fmt.Sprintf("%s-%s-%02d%s", fileWithoutExt, uniqueToken, idx, fileExt)
+
+		err = os.WriteFile(filename, content, 0600)
+		if err != nil {
+			return errors.Wrapf(err, "Failed write %s", filename)
+		}
+
+		log.Debugf("Created manifest file %s out of %s", filename, manifestPath)
+	}
+
+	err = os.Remove(manifestPath)
+	if err != nil {
+		return errors.Wrapf(err, "failed to remove multi-doc yaml %s", manifestPath)
+	}
+
+	return nil
+}
+
 func getHostnames(hosts []*models.Host) []string {
 	ret := make([]string, 0)
 	for _, h := range hosts {

diff --git a/internal/ignition/ignition_test.go b/internal/ignition/ignition_test.go
@@ -24,6 +24,7 @@ import (
 	. "github.com/onsi/ginkgo/extensions/table"
 	. "github.com/onsi/gomega"
 	"github.com/openshift/assisted-service/internal/common"
+	"github.com/openshift/assisted-service/internal/constants"
 	"github.com/openshift/assisted-service/internal/host/hostutil"
 	"github.com/openshift/assisted-service/internal/oc"
 	"github.com/openshift/assisted-service/internal/operators"
@@ -2010,6 +2011,128 @@ spec:
 	})
 })
 
+var _ = Describe("expand multi document yamls", func() {
+	var (
+		ctrl         *gomock.Controller
+		mockS3Client *s3wrapper.MockAPI
+		generator    *installerGenerator
+		ctx          = context.Background()
+	)
+
+	BeforeEach(func() {
+		ctrl = gomock.NewController(GinkgoT())
+		mockS3Client = s3wrapper.NewMockAPI(ctrl)
+		generator = &installerGenerator{
+			log:      log,
+			workDir:  workDir,
+			s3Client: mockS3Client,
+			cluster:  cluster,
+		}
+	})
+
+	AfterEach(func() {
+		ctrl.Finish()
+	})
+
+	It("yaml file is split when contains multiple documents", func() {
+		multiDocYaml := `---
+first: one
+---
+---
+- second: two
+---
+`
+		s3Metadata := []string{
+			filepath.Join(cluster.ID.String(), constants.ManifestMetadataFolder, "manifests", "multidoc.yml", constants.ManifestSourceUserSupplied),
+			filepath.Join(cluster.ID.String(), constants.ManifestMetadataFolder, "manifests", "manifest.json", constants.ManifestSourceUserSupplied), // json file will be ignored
+			filepath.Join(cluster.ID.String(), constants.ManifestMetadataFolder, "manifests", "manifest.yml", "other-metadata"),
+		}
+		mockS3Client.EXPECT().ListObjectsByPrefix(ctx, filepath.Join(cluster.ID.String(), constants.ManifestMetadataFolder)).Return(s3Metadata, nil).Times(1)
+
+		manifestsDir := filepath.Join(workDir, "/manifests")
+		Expect(os.Mkdir(manifestsDir, 0755)).To(Succeed())
+
+		err := os.WriteFile(filepath.Join(manifestsDir, "multidoc.yml"), []byte(multiDocYaml), 0600)
+		Expect(err).NotTo(HaveOccurred())
+
+		err = generator.expandUserMultiDocYamls(ctx)
+		Expect(err).To(Succeed())
+
+		entries, err := os.ReadDir(manifestsDir)
+		Expect(err).NotTo(HaveOccurred())
+		Expect(entries).To(HaveLen(2))
+
+		content, err := os.ReadFile(filepath.Join(manifestsDir, entries[0].Name()))
+		Expect(err).NotTo(HaveOccurred())
+		Expect(string(content)).To(Equal("first: one\n"))
+
+		content, err = os.ReadFile(filepath.Join(manifestsDir, entries[1].Name()))
+		Expect(err).NotTo(HaveOccurred())
+		Expect(string(content)).To(Equal("- second: two\n"))
+	})
+
+	It("file names contain a unique token when multi document yaml file is split", func() {
+		multiDocYaml := `---
+first: one
+---
+- second: two
+`
+		manifestsDir := filepath.Join(workDir, "/manifests")
+		Expect(os.Mkdir(manifestsDir, 0755)).To(Succeed())
+
+		manifestFilename := filepath.Join(manifestsDir, "multidoc.yml")
+		err := os.WriteFile(manifestFilename, []byte(multiDocYaml), 0600)
+		Expect(err).NotTo(HaveOccurred())
+
+		uniqueToken := "sometoken"
+		err = generator.expandMultiDocYaml(ctx, manifestFilename, uniqueToken)
+		Expect(err).To(Succeed())
+
+		entries, err := os.ReadDir(manifestsDir)
+		Expect(err).NotTo(HaveOccurred())
+		Expect(entries).To(HaveLen(2))
+
+		firstManifest := fmt.Sprintf("multidoc-%s-00.yml", uniqueToken)
+		content, err := os.ReadFile(filepath.Join(manifestsDir, firstManifest))
+		Expect(err).NotTo(HaveOccurred())
+		Expect(string(content)).To(Equal("first: one\n"))
+
+		secondManifest := fmt.Sprintf("multidoc-%s-01.yml", uniqueToken)
+		content, err = os.ReadFile(filepath.Join(manifestsDir, secondManifest))
+		Expect(err).NotTo(HaveOccurred())
+		Expect(string(content)).To(Equal("- second: two\n"))
+	})
+
+	It("yaml file is left untouched when contains one document", func() {
+		yamlDoc := `---
+first: one
+---
+`
+		s3Metadata := []string{
+			filepath.Join(cluster.ID.String(), constants.ManifestMetadataFolder, "openshift", "manifest.yml", constants.ManifestSourceUserSupplied),
+		}
+		mockS3Client.EXPECT().ListObjectsByPrefix(ctx, filepath.Join(cluster.ID.String(), constants.ManifestMetadataFolder)).Return(s3Metadata, nil).Times(1)
+
+		openshiftDir := filepath.Join(workDir, "/openshift")
+		Expect(os.Mkdir(openshiftDir, 0755)).To(Succeed())
+
+		manifestFilename := filepath.Join(openshiftDir, "manifest.yml")
+		err := os.WriteFile(manifestFilename, []byte(yamlDoc), 0600)
+		Expect(err).NotTo(HaveOccurred())
+
+		err = generator.expandUserMultiDocYamls(ctx)
+		Expect(err).To(Succeed())
+
+		entries, err := os.ReadDir(openshiftDir)
+		Expect(err).NotTo(HaveOccurred())
+		Expect(entries).To(HaveLen(1))
+
+		content, err := os.ReadFile(manifestFilename)
+		Expect(err).NotTo(HaveOccurred())
+		Expect(string(content)).To(Equal(yamlDoc))
+	})
+})
+
 var _ = Describe("Set kubelet node ip", func() {
 	var (
 		ctrl         *gomock.Controller