Skip to content

Commit

Permalink
refactor: remove legacy phrasing from Typescript CLI
Browse files Browse the repository at this point in the history
  • Loading branch information
thisislawatts committed Feb 26, 2024
1 parent e9723af commit 2a6864a
Show file tree
Hide file tree
Showing 8 changed files with 40 additions and 41 deletions.
4 changes: 2 additions & 2 deletions cliv2/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ test-signature: ISSIGNED_SCRIPT

# Typescript CLI targets
$(V1_WORKING_DIR)/$(V1_BINARY_FOLDER)/$(V1_EXECUTABLE_NAME):
@echo "$(LOG_PREFIX) Building legacy CLI"
@echo "$(LOG_PREFIX) Building Typescript CLI"
@cd $(V1_WORKING_DIR) && npm i && npm run $(V1_BUILD_TYPE)
@$(MAKE) -C $(V1_WORKING_DIR) $(V1_BINARY_FOLDER)/$(V1_EXECUTABLE_NAME) BINARY_RELEASES_FOLDER_TS_CLI=$(V1_BINARY_FOLDER)

Expand All @@ -222,7 +222,7 @@ generate-ls-protocol-metadata:

.PHONY: clean-ts-cli
clean-ts-cli:
@echo "$(LOG_PREFIX) Cleaning legacy CLI"
@echo "$(LOG_PREFIX) Cleaning Typescript CLI"
@$(MAKE) -C $(V1_WORKING_DIR) clean-ts BINARY_RELEASES_FOLDER_TS_CLI=$(V1_BINARY_FOLDER)

# build the full CLI (Typescript+Golang)
Expand Down
39 changes: 19 additions & 20 deletions cliv2/cmd/cliv2/main.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package main

// !!! This import needs to be the first import, please do not change this !!!
import _ "github.com/snyk/go-application-framework/pkg/networking/fips_enable"

import (
"context"
"encoding/json"
Expand All @@ -15,9 +13,6 @@ import (
"time"

"github.com/rs/zerolog"
"github.com/spf13/cobra"
"github.com/spf13/pflag"

"github.com/snyk/cli-extension-dep-graph/pkg/depgraph"
"github.com/snyk/cli-extension-iac-rules/iacrules"
"github.com/snyk/cli-extension-sbom/pkg/sbom"
Expand All @@ -26,18 +21,22 @@ import (
"github.com/snyk/go-application-framework/pkg/app"
"github.com/snyk/go-application-framework/pkg/auth"
"github.com/snyk/go-application-framework/pkg/configuration"
_ "github.com/snyk/go-application-framework/pkg/networking/fips_enable"
"github.com/spf13/cobra"
"github.com/spf13/pflag"

"github.com/snyk/cli/cliv2/internal/cliv2"
"github.com/snyk/cli/cliv2/internal/constants"
"github.com/snyk/cli/cliv2/pkg/basic_workflows"
localworkflows "github.com/snyk/go-application-framework/pkg/local_workflows"
"github.com/snyk/go-application-framework/pkg/networking"
"github.com/snyk/go-application-framework/pkg/runtimeinfo"
"github.com/snyk/go-application-framework/pkg/utils"
"github.com/snyk/go-application-framework/pkg/workflow"
"github.com/snyk/go-httpauth/pkg/httpauth"
"github.com/snyk/snyk-iac-capture/pkg/capture"
snykls "github.com/snyk/snyk-ls/ls_extension"

"github.com/snyk/cli/cliv2/internal/cliv2"
"github.com/snyk/cli/cliv2/internal/constants"
"github.com/snyk/cli/cliv2/pkg/basic_workflows"
snykls "github.com/snyk/snyk-ls/ls_extension"
)

var internalOS string
Expand All @@ -62,9 +61,9 @@ type JsonErrorStruct struct {
type HandleError int

const (
handleErrorFallbackToLegacyCLI HandleError = iota
handleErrorShowHelp HandleError = iota
handleErrorUnhandled HandleError = iota
handleErrorFallbackTotypescriptcli HandleError = iota
handleErrorShowHelp HandleError = iota
handleErrorUnhandled HandleError = iota
)

func main() {
Expand Down Expand Up @@ -194,7 +193,7 @@ func help(_ *cobra.Command, args []string) error {
}

func defaultCmd(args []string) error {
// prepare the invocation of the legacy CLI by
// prepare the invocation of the Typescript CLI by
// * enabling stdio
// * by specifying the raw cmd args for it
globalConfiguration.Set(configuration.WORKFLOW_USE_STDIO, true)
Expand Down Expand Up @@ -267,7 +266,7 @@ func prepareRootCommand() *cobra.Command {
},
}

// help for all commands is handled by the legacy cli
// help for all commands is handled by the Typescript CLI
// TODO: discuss how to move help to extensions
helpCommand := cobra.Command{
Use: "help",
Expand All @@ -280,7 +279,7 @@ func prepareRootCommand() *cobra.Command {
rootCommand.SilenceUsage = true
rootCommand.FParseErrWhitelist.UnknownFlags = true

// ensure that help and usage information comes from the legacy cli instead of cobra's default help
// ensure that help and usage information comes from the Typescript CLI instead of cobra's default help
rootCommand.SetHelpFunc(func(c *cobra.Command, args []string) { _ = help(c, args) })
rootCommand.SetHelpCommand(&helpCommand)
rootCommand.PersistentFlags().AddFlagSet(getGlobalFLags())
Expand All @@ -306,9 +305,9 @@ func handleError(err error) HandleError {

// filter for known cobra errors, since cobra errors shall trigger a fallback, but not others.
if commandError {
resultError = handleErrorFallbackToLegacyCLI
resultError = handleErrorFallbackTotypescriptcli
} else if flagError {
// handle flag errors explicitly since we need to delegate the help to the legacy CLI. This includes disabling the cobra default help/usage
// handle flag errors explicitly since we need to delegate the help to the Typescript CLI. This includes disabling the cobra default help/usage
resultError = handleErrorShowHelp
}
}
Expand Down Expand Up @@ -421,10 +420,10 @@ func MainWithErrorCode() int {
// run the extensible cli
err = rootCommand.Execute()

// fallback to the legacy cli or show help
// fallback to the Typescript CLI or show help
handleErrorResult := handleError(err)
if handleErrorResult == handleErrorFallbackToLegacyCLI {
globalLogger.Printf("Using Legacy CLI to serve the command. (reason: %v)", err)
if handleErrorResult == handleErrorFallbackTotypescriptcli {
globalLogger.Printf("Using Typescript CLI to serve the command. (reason: %v)", err)
err = defaultCmd(os.Args[1:])
} else if handleErrorResult == handleErrorShowHelp {
err = help(nil, []string{})
Expand Down
4 changes: 2 additions & 2 deletions cliv2/cmd/cliv2/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,8 @@ func Test_CreateCommandsForWorkflowWithSubcommands(t *testing.T) {
assert.Equal(t, handleErrorUnhandled, handleError(subcmd2.RunE(subcmd2, []string{})))
assert.Equal(t, handleErrorUnhandled, handleError(subcmd3.RunE(subcmd3, []string{})))
assert.Equal(t, handleErrorUnhandled, handleError(something.RunE(something, []string{})))
assert.Equal(t, handleErrorFallbackToLegacyCLI, handleError(subcmd1.RunE(subcmd1, []string{})))
assert.Equal(t, handleErrorFallbackToLegacyCLI, handleError(cmd2.RunE(cmd2, []string{})))
assert.Equal(t, handleErrorFallbackTotypescriptcli, handleError(subcmd1.RunE(subcmd1, []string{})))
assert.Equal(t, handleErrorFallbackTotypescriptcli, handleError(cmd2.RunE(cmd2, []string{})))
assert.Equal(t, handleErrorShowHelp, handleError(parseError))

assert.True(t, subcmd1.DisableFlagParsing)
Expand Down
2 changes: 1 addition & 1 deletion cliv2/internal/cliv2/cliv2.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ func (c *CLI) ExtractV1Binary() error {
c.DebugLogger.Println("Extracted cliv1 successfully")
} else {
c.DebugLogger.Println("Extracted cliv1 is not valid")
return fmt.Errorf("failed to extract legacy cli")
return fmt.Errorf("failed to extract Typescript CLI")
}
} else {
c.DebugLogger.Println("Extraction not required")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,21 +20,21 @@ import (
"github.com/snyk/cli/cliv2/internal/proxy"
)

var WORKFLOWID_LEGACY_CLI workflow.Identifier = workflow.NewWorkflowIdentifier("legacycli")
var WORKFLOWID_LEGACY_CLI workflow.Identifier = workflow.NewWorkflowIdentifier("typescriptcli")
var DATATYPEID_LEGACY_CLI_STDOUT workflow.Identifier = workflow.NewTypeIdentifier(WORKFLOWID_LEGACY_CLI, "stdout")

const (
PROXY_NOAUTH string = "proxy-noauth"
)

func Init(engine workflow.Engine) error {
flagset := pflag.NewFlagSet("legacycli", pflag.ContinueOnError)
flagset.StringSlice(configuration.RAW_CMD_ARGS, os.Args[1:], "Command line arguments for the legacy CLI.")
flagset := pflag.NewFlagSet("typescriptcli", pflag.ContinueOnError)
flagset.StringSlice(configuration.RAW_CMD_ARGS, os.Args[1:], "Command line arguments for the Typescript CLI.")
flagset.Bool(configuration.WORKFLOW_USE_STDIO, false, "Use StdIn and StdOut")
flagset.String(configuration.WORKING_DIRECTORY, "", "CLI working directory")

config := workflow.ConfigurationOptionsFromFlagset(flagset)
entry, _ := engine.Register(WORKFLOWID_LEGACY_CLI, config, legacycliWorkflow)
entry, _ := engine.Register(WORKFLOWID_LEGACY_CLI, config, typescriptcliWorkflow)
entry.SetVisibility(false)
return nil
}
Expand All @@ -55,7 +55,7 @@ func finalizeArguments(args []string, unknownArgs []string) []string {
return filteredArgs
}

func legacycliWorkflow(
func typescriptcliWorkflow(
invocation workflow.InvocationContext,
_ []workflow.Data,
) (output []workflow.Data, err error) {
Expand Down Expand Up @@ -98,9 +98,9 @@ func legacycliWorkflow(
}

if oauthIsAvailable {
// The Legacy CLI doesn't support oauth authentication. Oauth authentication is implemented in the Extensible CLI and is added
// to the legacy CLI by forwarding network traffic through the internal proxy of the Extensible CLI.
// The legacy CLI always expects some sort of token to be available, otherwise some functionality isn't available. This is why we inject
// The Typescript CLI doesn't support oauth authentication. Oauth authentication is implemented in the Extensible CLI and is added
// to the Typescript CLI by forwarding network traffic through the internal proxy of the Extensible CLI.
// The Typescript CLI always expects some sort of token to be available, otherwise some functionality isn't available. This is why we inject
// a random token value to bypass these checks and replace the proper authentication headers in the internal proxy.
// Injecting the real token here and not in the proxy would create an issue when the token expires during CLI execution.
if oauth := config.GetString(auth.CONFIG_KEY_OAUTH_TOKEN); len(oauth) > 0 {
Expand Down
File renamed without changes.
12 changes: 6 additions & 6 deletions ts-binary-wrapper/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import * as path from 'path';

const errorContextMessage = 'Runtime';
const fallbackScript = path.join(__dirname, '..', 'dist', 'cli', 'index.js');
const legacyCLIflag = '--legacy-cli';
const typescriptCliFlag = '--legacy-cli';

function run(executable: string): number {
let cliArguments = common.getCliArguments(argv);
Expand All @@ -22,9 +22,9 @@ function run(executable: string): number {
}

(async () => {
let fallbackToLegacyCLI = argv.includes(legacyCLIflag);
let fallbackToTypescriptCli = argv.includes(typescriptCliFlag);

if (fallbackToLegacyCLI === false) {
if (fallbackToTypescriptCli === false) {
try {
const config = common.getCurrentConfiguration();
const executable = config.getLocalLocation();
Expand All @@ -45,19 +45,19 @@ function run(executable: string): number {
// run the executable
process.exit(run(executable));
} catch (error) {
fallbackToLegacyCLI = true;
fallbackToTypescriptCli = true;
await common.logError(errorContextMessage, error);
}
} else {
// if --legacy-clli is enabled create a log messaeg
await common.logError(
errorContextMessage,
Error(legacyCLIflag + ' is set'),
Error(typescriptCliFlag + ' is set'),
false,
);
}

if (fallbackToLegacyCLI) {
if (fallbackToTypescriptCli) {
common.formatErrorMessage('legacy-cli');
const exitCode = run(fallbackScript);
common.formatErrorMessage('legacy-cli');
Expand Down
4 changes: 2 additions & 2 deletions ts-binary-wrapper/test/acceptance/basic.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ describe('Basic acceptance test', () => {
);

expect(fs.existsSync(executable)).toBeFalsy();
expect(resultIndex.status).not.toEqual(0); // we expect this to fail, since the legacy cli is not available in the test, still we want to see that the logic around is working properly
expect(resultIndex.status).not.toEqual(0); // we expect this to fail, since the Typescript CLI is not available in the test, still we want to see that the logic around is working properly
expect(resultIndex.stdout.toString()).toEqual('');
expect(resultIndex.stderr.toString()).toContain(
'You are currently running a degraded version of the Snyk CLI.',
Expand Down Expand Up @@ -173,7 +173,7 @@ describe('Basic acceptance test', () => {
);

expect(fs.existsSync(executable)).toBeFalsy();
expect(resultIndex.status).not.toEqual(0); // we expect this to fail, since the legacy cli is not available in the test, still we want to see that the logic around is working properly
expect(resultIndex.status).not.toEqual(0); // we expect this to fail, since the Typescript CLI is not available in the test, still we want to see that the logic around is working properly
expect(resultIndex.stdout.toString()).toEqual('');
expect(resultIndex.stderr.toString()).toContain(
'You are currently running a degraded version of the Snyk CLI.',
Expand Down

0 comments on commit 2a6864a

Please sign in to comment.