Merge pull request #283 from Kostov6/vars_and_flags_refactor

Vars and flags refactor

.ci/integration-test

@@ -121,12 +121,12 @@ if [[ -z "${org}" ]] || [[ -z "${mainBranch}" ]]; then
   exit 1
 fi
 
-GIT_OAUTH_TOKEN=${GIT_OAUTH_TOKEN:-$(getGitHubToken github_com)}
+GIT_OAUTH_TOKEN="github.com=${GITHUB_OAUTH_TOKEN:-$(getGitHubToken github_com)}"
 test "$GIT_OAUTH_TOKEN" #fail fast
 
 # Run docforge command with Git handler
 echo "Run ${docforge_bin}"
-${docforge_bin} -f "${int_test_manifest}" -d "${int_test_output_tree_dir}" --hugo --github-oauth-token "${GIT_OAUTH_TOKEN}" --github-info-destination ../generated-metadata --variables "org=${org},version=${mainBranch}"
+${docforge_bin} -f "${int_test_manifest}" -d "${int_test_output_tree_dir}" --hugo --github-oauth-token-map "${GIT_OAUTH_TOKEN}" --github-info-destination ../generated-metadata
 
 #Remove untested metadata keys
 removeUntestedKeysFromMetadata "${int_test_expected_metadata_dir}"

cmd/app/cmd.go

@@ -34,20 +34,17 @@ type Options struct {
 	DocumentationManifestPath    string            `mapstructure:"manifest"`
 	ResourcesPath                string            `mapstructure:"resources-download-path"`
 	ResourceDownloadWorkersCount int               `mapstructure:"download-workers"`
-	Variables                    map[string]string `mapstructure:"variables"` // TODO: get rid of this option
 	GhInfoDestination            string            `mapstructure:"github-info-destination"`
 	DryRun                       bool              `mapstructure:"dry-run"`
 	Resolve                      bool              `mapstructure:"resolve"` // TODO: use-case for this option ??
 	Hugo                         bool              `mapstructure:"hugo"`
 	HugoPrettyUrls               bool              `mapstructure:"hugo-pretty-urls"` // TODO: hugo defaults to pretty urls -> make sense to use 'hugo-ugly-urls' instead
 	FlagsHugoSectionFiles        []string          `mapstructure:"hugo-section-files"`
 	HugoBaseURL                  string            `mapstructure:"hugo-base-url"`
-	UseGit                       bool              `mapstructure:"use-git"` // TODO: get rid of this option
 	CacheHomeDir                 string            `mapstructure:"cache-dir"`
-	Credentials                  []Credential      `mapstructure:"credentials"` // TODO: one way to provide credentials (e.g. use only 'github-oauth-token-map')
+	Credentials                  []Credential      `mapstructure:"credentials"`
 	ResourceMappings             map[string]string `mapstructure:"resourceMappings"`
-	GhOAuthToken                 string            `mapstructure:"github-oauth-token"`     // TODO: one way to provide credentials
-	GhOAuthTokens                map[string]string `mapstructure:"github-oauth-token-map"` // TODO: one way to provide credentials
+	GhOAuthTokens                map[string]string `mapstructure:"github-oauth-token-map"`
 }
 
 //Credential holds repository credential data
@@ -81,7 +78,7 @@ func NewCommand(ctx context.Context) *cobra.Command {
 			if rhs, err = initResourceHandlers(ctx, options); err != nil {
 				return err
 			}
-			if doc, err = manifest(ctx, options.DocumentationManifestPath, rhs, options.Variables, options.Hugo); err != nil {
+			if doc, err = manifest(ctx, options.DocumentationManifestPath, rhs, options.Hugo); err != nil {
 				return err
 			}
 			reactor, err := NewReactor(options, rhs)
@@ -143,10 +140,6 @@ func configureFlags(command *cobra.Command) {
 		"Resources download path.")
 	_ = vip.BindPFlag("resources-download-path", command.Flags().Lookup("resources-download-path"))
 
-	command.Flags().String("github-oauth-token", "",
-		"GitHub personal token authorizing read access from GitHub.com repositories. For authorization credentials for multiple GitHub instances, see --github-oauth-token-map")
-	_ = vip.BindPFlag("github-oauth-token", command.Flags().Lookup("github-oauth-token"))
-
 	command.Flags().StringToString("github-oauth-token-map", map[string]string{},
 		"GitHub personal tokens authorizing read access from repositories per GitHub instance. Note that if the GitHub token is already provided by `github-oauth-token` it will be overridden by it.")
 	_ = vip.BindPFlag("github-oauth-token-map", command.Flags().Lookup("github-oauth-token-map"))
@@ -155,10 +148,6 @@ func configureFlags(command *cobra.Command) {
 		"If specified, docforge will download also additional github info for the files from the documentation structure into this destination.")
 	_ = vip.BindPFlag("github-info-destination", command.Flags().Lookup("github-info-destination"))
 
-	command.Flags().StringToString("variables", map[string]string{},
-		"Variables applied to parameterized (using Go template) manifest.")
-	_ = vip.BindPFlag("variables", command.Flags().Lookup("variables"))
-
 	command.Flags().Bool("fail-fast", false,
 		"Fail-fast vs fault tolerant operation.")
 	_ = vip.BindPFlag("fail-fast", command.Flags().Lookup("fail-fast"))
@@ -195,10 +184,6 @@ func configureFlags(command *cobra.Command) {
 		"Rewrites the relative links of documentation files to root-relative where possible.")
 	_ = vip.BindPFlag("hugo-base-url", command.Flags().Lookup("hugo-base-url"))
 
-	command.Flags().Bool("use-git", false,
-		"Use Git for replication")
-	_ = vip.BindPFlag("use-git", command.Flags().Lookup("use-git"))
-
 	command.Flags().StringSlice("hugo-section-files", []string{"readme.md", "readme", "read.me", "index.md", "index"},
 		"When building a Hugo-compliant documentation bundle, files with filename matching one form this list (in that order) will be renamed to _index.md. Only useful with --hugo=true")
 	_ = vip.BindPFlag("hugo-section-files", command.Flags().Lookup("hugo-section-files"))
@@ -261,7 +246,6 @@ func gatherCredentials() []Credential {
 		klog.Warningf("error in unmarshalling credentials from config: %s", err.Error())
 	}
 	ghOAuthTokens := vip.GetStringMapString("github-oauth-token-map")
-	ghOAuthToken := vip.GetString("github-oauth-token")
 
 	credentialsByHost := make(map[string]Credential)
 
@@ -293,35 +277,16 @@ func gatherCredentials() []Credential {
 		}
 	}
 
-	if len(ghOAuthToken) > 0 {
-		//provided ghOAuthToken may override credentialsByHost. This is the default logic
-		var username string
-		token := ghOAuthToken
-		if _, ok := credentialsByHost["github.com"]; ok {
-			klog.Warning("github.com token is overridden by the provided token with `--github-oauth-token flag`\n")
-		}
-		usernameAndToken := strings.Split(ghOAuthToken, ":")
-		if len(usernameAndToken) == 2 {
-			username = usernameAndToken[0]
-			token = usernameAndToken[1]
-		}
-
+	if _, ok := credentialsByHost["github.com"]; !ok {
+		klog.Infof("using unauthenticated github access\n")
+		//credentialByHost at github.com is not set and should be set to empty string
 		credentialsByHost["github.com"] = Credential{
 			Host:       "github.com",
-			Username:   username,
-			OAuthToken: token,
-		}
-	} else {
-		if _, ok := credentialsByHost["github.com"]; !ok {
-			klog.Infof("using unauthenticated github access\n")
-			//credentialByHost at github.com is not set and should be set to empty string
-			credentialsByHost["github.com"] = Credential{
-				Host:       "github.com",
-				Username:   "",
-				OAuthToken: "",
-			}
+			Username:   "",
+			OAuthToken: "",
 		}
 	}
+
 	var credentials = make([]Credential, 0, len(credentialsByHost))
 	for _, cred := range credentialsByHost {
 		credentials = append(credentials, cred)

cmd/app/factory.go

@@ -91,26 +91,20 @@ func initResourceHandlers(ctx context.Context, o *Options) ([]resourcehandlers.R
 		if err != nil {
 			errs = multierror.Append(errs, err)
 		}
-		rh := newResourceHandler(u.Host, o.CacheHomeDir, &cred.Username, cred.OAuthToken, client, httpClient, o.UseGit, o.ResourceMappings, o.Variables, o.Hugo)
+		rh := newResourceHandler(u.Host, o.CacheHomeDir, &cred.Username, cred.OAuthToken, client, httpClient, o.ResourceMappings, o.Hugo)
 		rhs = append(rhs, rh)
 	}
 
 	return rhs, errs.ErrorOrNil()
 }
 
 // TODO: remove unused params
-func newResourceHandler(host, homeDir string, user *string, token string, client *github.Client, httpClient *http.Client, useGit bool, localMappings map[string]string, flagVars map[string]string, hugoEnabled bool) resourcehandlers.ResourceHandler {
+func newResourceHandler(host, homeDir string, user *string, token string, client *github.Client, httpClient *http.Client, localMappings map[string]string, hugoEnabled bool) resourcehandlers.ResourceHandler {
 	rawHost := "raw." + host
 	if host == "github.com" {
 		rawHost = "raw.githubusercontent.com"
 	}
-
-	//	if useGit { TODO: remove unused resource handlers
-	//		return git.NewResourceHandler(filepath.Join(homeDir, git.CacheDir), user, token, client, httpClient, []string{host, rawHost}, localMappings, branchesMap, flagVars)
-	//	}
-	//	return ghrs.NewResourceHandler(client, httpClient, []string{host, rawHost}, branchesMap, flagVars)
-
-	return pg.NewPG(client, httpClient, &osshim.OsShim{}, []string{host, rawHost}, localMappings, flagVars, hugoEnabled)
+	return pg.NewPG(client, httpClient, &osshim.OsShim{}, []string{host, rawHost}, localMappings, hugoEnabled)
 }
 
 func buildClient(ctx context.Context, accessToken string, host string, cachePath string) (*github.Client, *http.Client, error) {

cmd/app/manifest.go

@@ -17,7 +17,7 @@ import (
 
 // Manifest reads the resource at uri, resolves it as template applying vars,
 // and finally parses it into api.Documentation model
-func manifest(ctx context.Context, uri string, resourceHandlers []resourcehandlers.ResourceHandler, flagsVars map[string]string, hugoEnabled bool) (*api.Documentation, error) {
+func manifest(ctx context.Context, uri string, resourceHandlers []resourcehandlers.ResourceHandler, hugoEnabled bool) (*api.Documentation, error) {
 	var (
 		handler         resourcehandlers.ResourceHandler
 		manifestContent []byte
@@ -38,7 +38,7 @@ func manifest(ctx context.Context, uri string, resourceHandlers []resourcehandle
 			return nil, err
 		}
 
-		doc, err := api.ParseWithMetadata(manifestContent, "master", flagsVars, hugoEnabled)
+		doc, err := api.Parse(manifestContent, hugoEnabled)
 		if err != nil {
 			return nil, fmt.Errorf("failed to parse manifest: %s. %+v", uri, err)
 		}

integration-test/manifest.yaml

@@ -1,35 +1,33 @@
 # SPDX-FileCopyrightText: 2021 SAP SE or an SAP affiliate company and Gardener contributors
 #
 # SPDX-License-Identifier: Apache-2.0
-{{$org := .org}}
-{{$version := .version}}
 structure:
 - name: mainTree
   nodes:
   - name: markdown-tests
     nodes:
-    - source: https://github.com/{{$org}}/docforge/blob/{{$version}}/integration-test/tested-doc/markdown-tests/testedMarkdownFile1.md
+    - source: https://github.com/gardener/docforge/blob/master/integration-test/tested-doc/markdown-tests/testedMarkdownFile1.md
       name: file1.md
-    - source: https://github.com/{{$org}}/docforge/blob/{{$version}}/integration-test/tested-doc/markdown-tests/testedMarkdownFile2.md
+    - source: https://github.com/gardener/docforge/blob/master/integration-test/tested-doc/markdown-tests/testedMarkdownFile2.md
     - name: nodeSelector
       properties:
         frontmatter:
           title: NodeSelector
           description: Test NodeSelector
       nodesSelector:
-        path: https://github.com/{{$org}}/docforge/tree/{{$version}}/integration-test/tested-doc/markdown-tests/testedDir
+        path: https://github.com/gardener/docforge/tree/master/integration-test/tested-doc/markdown-tests/testedDir
   - name: html-tests
     nodes:
-    - source: https://github.com/{{$org}}/docforge/blob/{{$version}}/integration-test/tested-doc/html-tests/testedHTMLFile1.md
+    - source: https://github.com/gardener/docforge/blob/master/integration-test/tested-doc/html-tests/testedHTMLFile1.md
       name: file1.md
-    - source: https://github.com/{{$org}}/docforge/blob/{{$version}}/integration-test/tested-doc/html-tests/testedHTMLFile2.md
+    - source: https://github.com/gardener/docforge/blob/master/integration-test/tested-doc/html-tests/testedHTMLFile2.md
     - name: nodeSelector
       properties:
         frontmatter:
           title: NodeSelector
           description: Test NodeSelector
       nodesSelector:
-        path: https://github.com/{{$org}}/docforge/tree/{{$version}}/integration-test/tested-doc/html-tests/testedDir
+        path: https://github.com/gardener/docforge/tree/master/integration-test/tested-doc/html-tests/testedDir
 - name: merge-node
   nodes:
   - name: level1-container-node1
@@ -39,18 +37,18 @@ structure:
       - name: level3-container-node1
         nodes:
         - name: file1
-          source: https://github.com/{{$org}}/docforge/blob/{{$version}}/integration-test/tested-doc/merge-test/testFile.md
+          source: https://github.com/gardener/docforge/blob/master/integration-test/tested-doc/merge-test/testFile.md
         - name: file2
-          source: https://github.com/{{$org}}/docforge/blob/{{$version}}/integration-test/tested-doc/merge-test/testFile.md
+          source: https://github.com/gardener/docforge/blob/master/integration-test/tested-doc/merge-test/testFile.md
     - name: level2-container-node2
       nodes:
       - name: file1
-        source: https://github.com/{{$org}}/docforge/blob/{{$version}}/integration-test/tested-doc/merge-test/testFile.md
+        source: https://github.com/gardener/docforge/blob/master/integration-test/tested-doc/merge-test/testFile.md
   - name: level1-container-node2
     nodes:
     - name: file1
-      source: https://github.com/{{$org}}/docforge/blob/{{$version}}/integration-test/tested-doc/merge-test/testFile.md
+      source: https://github.com/gardener/docforge/blob/master/integration-test/tested-doc/merge-test/testFile.md
   nodesSelector:
-    path: https://github.com/{{$org}}/docforge/blob/{{$version}}/integration-test/tested-doc/merge-test/testSelector.yaml
+    path: https://github.com/gardener/docforge/blob/master/integration-test/tested-doc/merge-test/testSelector.yaml
 nodesSelector:
-  path: https://github.com/{{$org}}/docforge/tree/{{$version}}/integration-test/tested-doc/merge-test/testDir
+  path: https://github.com/gardener/docforge/tree/master/integration-test/tested-doc/merge-test/testDir

pkg/api/parser.go

@@ -17,24 +17,11 @@ import (
 	"gopkg.in/yaml.v3"
 )
 
-// ParseWithMetadata parses a document's byte content given some other metainformation
-func ParseWithMetadata(b []byte, targetBranch string, flagsVars map[string]string, hugoEnabled bool) (*Documentation, error) {
-	versionList := make([]string, 0)
-	versionList = append(versionList, targetBranch)
-
-	versions := strings.Join(versionList, ",")
-	flagsVars["versions"] = versions
-	return Parse(b, flagsVars, hugoEnabled)
-}
-
 // Parse is a function which construct documentation struct from given byte array
-func Parse(b []byte, flagsVars map[string]string, hugoEnabled bool) (*Documentation, error) {
-	blob, err := resolveVariables(b, flagsVars)
-	if err != nil {
-		return nil, err
-	}
-	var docs = &Documentation{}
-	if err = yaml.Unmarshal(blob, docs); err != nil {
+func Parse(b []byte, hugoEnabled bool) (*Documentation, error) {
+	var err error
+	docs := &Documentation{}
+	if err = yaml.Unmarshal(b, docs); err != nil {
 		return nil, err
 	}
 	// init parents

pkg/api/parser_test.go

@@ -18,7 +18,7 @@ import (
 var _ = Describe("Parser", func() {
 	Describe("Parsing manifest", func() {
 		DescribeTable("parsing tests", func(manifest []byte, expDoc *api.Documentation, expErr error) {
-			doc, err := api.Parse(manifest, map[string]string{}, true)
+			doc, err := api.Parse(manifest, true)
 			if expErr == nil {
 				Expect(err).To(BeNil())
 			} else {
@@ -123,47 +123,6 @@ var _ = Describe("Parser", func() {
 				}, nil),
 		)
 	})
-	Describe("Parsing with metadata", func() {
-		var (
-			manifest     []byte
-			targetBranch string
-			got          *api.Documentation
-			err          error
-		)
-		JustBeforeEach(func() {
-			vars := map[string]string{}
-
-			got, err = api.ParseWithMetadata(manifest, targetBranch, vars, true)
-		})
-		Context("given a general use case", func() {
-			BeforeEach(func() {
-				manifest = []byte(`structure:
-- name: community
-  source: https://github.com/gardener/docforge/edit/master/integration-test/tested-doc/merge-test/testFile.md
-- name: docs
-  source: https://github.com/gardener/docforge/blob/master/integration-test/tested-doc/merge-test/testFile.md
-`)
-				targetBranch = "master"
-			})
-			It("should work as expected", func() {
-
-				Expect(err).NotTo(HaveOccurred())
-				Expect(got).To(Equal(&api.Documentation{
-					Structure: []*api.Node{
-						{
-							Name:   "community",
-							Source: "https://github.com/gardener/docforge/edit/master/integration-test/tested-doc/merge-test/testFile.md",
-						},
-						{
-							Name:   "docs",
-							Source: "https://github.com/gardener/docforge/blob/master/integration-test/tested-doc/merge-test/testFile.md",
-						},
-					},
-				}))
-			})
-		})
-
-	})
 	Describe("Serialize", func() {
 		var (
 			doc *api.Documentation
@@ -236,7 +195,7 @@ var _ = Describe("Parser", func() {
 			exp      *api.Documentation
 		)
 		JustBeforeEach(func() {
-			got, err = api.Parse(manifest, map[string]string{}, true)
+			got, err = api.Parse(manifest, true)
 		})
 		Context("given manifest file", func() {
 			BeforeEach(func() {

pkg/resourcehandlers/git/git_resource_handler.go

@@ -509,7 +509,7 @@ func (g *Git) ResolveDocumentation(ctx context.Context, uri string) (*api.Docume
 		return nil, nil
 	}
 	//DEPRECATED!!! if to be returned should get hugo par
-	doc, err := api.ParseWithMetadata(blob, rl.SHAAlias, g.flagVars, true)
+	doc, err := api.Parse(blob, true)
 	if err != nil {
 		return nil, fmt.Errorf("failed to parse manifest: %s. %+v", uri, err)
 	}

pkg/resourcehandlers/git/git_resource_handler_test.go

@@ -851,15 +851,9 @@ structure:
 structure:
 - name: community
   source: https://github.com/gardener/docforge/edit/master/integration-test/tested-doc/merge-test/testFile.md
-{{- $vers := Split .versions "," -}}
-{{- range $i, $version := $vers -}}
-{{- if eq $i 0  }}
 - name: docs
-{{- else }}
-- name: {{$version}}
-{{- end }}
-  source: https://github.com/gardener/docforge/blob/{{$version}}/integration-test/tested-doc/merge-test/testFile.md
-{{- end }}`))
+  source: https://github.com/gardener/docforge/blob/testMainBranch/integration-test/tested-doc/merge-test/testFile.md
+`))
 			fakeFileSystem.ReadFileReturns(manifestData, nil)
 
 		})

pkg/resourcehandlers/github/github_resource_handler.go

@@ -383,7 +383,7 @@ func (gh *GitHub) ResolveDocumentation(ctx context.Context, path string) (*api.D
 		return nil, err
 	}
 	//DEPRECATED!!! if to be returned should get hugo par
-	doc, err := api.ParseWithMetadata(blob, rl.SHAAlias, gh.flagVars, true)
+	doc, err := api.Parse(blob, true)
 	if err != nil {
 		return nil, fmt.Errorf("failed to parse manifest: %s. %+v", path, err)
 	}