From 025862689ec8dc460a1ef6f4402151c18a072ba3 Mon Sep 17 00:00:00 2001 From: Zbynek Roubalik <726523+zroubalik@users.noreply.github.com> Date: Thu, 24 Sep 2020 15:52:22 +0200 Subject: [PATCH] feat: decouple function name from function domain (#127) * decouple function name from function domain Signed-off-by: Zbynek Roubalik --- client.go | 17 +++----- client_test.go | 78 +++++++++------------------------- cmd/create.go | 18 ++++---- cmd/init.go | 29 +++++++------ cmd/root.go | 32 +++++++++++--- docs/commands.md | 2 +- function.go | 99 +++---------------------------------------- function_unit_test.go | 76 --------------------------------- k8s/names.go | 30 ++++++++----- k8s/names_test.go | 24 +++++------ knative/deployer.go | 21 +-------- knative/describer.go | 4 +- knative/lister.go | 2 +- knative/remover.go | 2 +- knative/updater.go | 4 +- 15 files changed, 126 insertions(+), 312 deletions(-) delete mode 100644 function_unit_test.go diff --git a/client.go b/client.go index 5ae57248bb..4900729b81 100644 --- a/client.go +++ b/client.go @@ -11,7 +11,6 @@ const ( DefaultRegistry = "docker.io" DefaultRuntime = "go" DefaultTrigger = "http" - DefaultMaxRecursion = 5 // when determining a name from path ) // Client for managing Function instances. @@ -127,7 +126,6 @@ func New(options ...Option) *Client { lister: &noopLister{output: os.Stdout}, dnsProvider: &noopDNSProvider{output: os.Stdout}, progressListener: &noopProgressListener{}, - domainSearchLimit: DefaultMaxRecursion, // no recursion limit deriving domain by default. } // Apply passed options, which take ultimate precidence. @@ -306,6 +304,12 @@ func (c *Client) Create(cfg Function) (err error) { // Initialize creates a new Function project locally using the settings // provided on a Function object. func (c *Client) Initialize(cfg Function) (err error) { + + // Create project root directory, if it doesn't already exist + if err = os.MkdirAll(cfg.Root, 0755); err != nil { + return + } + // Create Function of the given root path. f, err := NewFunction(cfg.Root) if err != nil { @@ -320,15 +324,8 @@ func (c *Client) Initialize(cfg Function) (err error) { f.Image = cfg.Image - // Set the name to that provided, defaulting to path derivation if empty. + // Set the name to that provided. f.Name = cfg.Name - if cfg.Name == "" { - f.Name = pathToDomain(f.Root, c.domainSearchLimit) - if f.Name == "" { - err = errors.New("Function name must be deriveable from path or explicitly provided") - return - } - } // Assert runtime was provided, or default. f.Runtime = cfg.Runtime diff --git a/client_test.go b/client_test.go index 35b5130f9a..e2e50d8d89 100644 --- a/client_test.go +++ b/client_test.go @@ -56,8 +56,14 @@ func TestCreateWritesTemplate(t *testing.T) { // TestCreateInitializedAborts ensures that a directory which contains an initialized // function does not reinitialize func TestCreateInitializedAborts(t *testing.T) { - root := "testdata/example.com/testCreateInitializedAborts" // contains only a .faas.config + root := "testdata/example.com/testCreateInitializedAborts" + defer os.RemoveAll(root) + client := faas.New() + if err := client.Initialize(faas.Function{Root: root}); err != nil { + t.Fatal(err) + } + if err := client.Initialize(faas.Function{Root: root}); err == nil { t.Fatal("error expected initilizing a path already containing an initialized Function") } @@ -67,6 +73,15 @@ func TestCreateInitializedAborts(t *testing.T) { // files aborts. func TestCreateNonemptyDirectoryAborts(t *testing.T) { root := "testdata/example.com/testCreateNonemptyDirectoryAborts" // contains only a single visible file. + if err := os.MkdirAll(root, 0744); err != nil { + t.Fatal(err) + } + defer os.RemoveAll(root) + _, err := os.Create(root + "/file.txt") + if err != nil { + t.Fatal(err) + } + client := faas.New() if err := client.Initialize(faas.Function{Root: root}); err == nil { t.Fatal("error expected initilizing a Function in a nonempty directory") @@ -212,57 +227,6 @@ func TestUnsupportedRuntime(t *testing.T) { } } -// TestDeriveDomain ensures that the name of the service is a domain derived -// from the current path if possible. -// see unit tests on the pathToDomain for more detailed logic. -func TestDeriveName(t *testing.T) { - // Create the root Function directory - root := "testdata/example.com/testDeriveDomain" - if err := os.MkdirAll(root, 0700); err != nil { - t.Fatal(err) - } - defer os.RemoveAll(root) - - client := faas.New(faas.WithRepository(TestRepository)) - if err := client.Create(faas.Function{Root: root}); err != nil { - t.Fatal(err) - } - - f, err := faas.NewFunction(root) - if err != nil { - t.Fatal(err) - } - - if f.Name != "testDeriveDomain.example.com" { - t.Fatalf("unexpected function name '%v'", f.Name) - } -} - -// TestDeriveSubdomans ensures that a subdirectory structure is interpreted as -// multilevel subdomains when calculating a derived name for a service. -func TestDeriveSubdomains(t *testing.T) { - // Create the test Function root - root := "testdata/example.com/region1/testDeriveSubdomains" - if err := os.MkdirAll(root, 0700); err != nil { - t.Fatal(err) - } - defer os.RemoveAll(root) - - client := faas.New(faas.WithRepository(TestRepository)) - if err := client.Create(faas.Function{Root: root}); err != nil { - t.Fatal(err) - } - - f, err := faas.NewFunction(root) - if err != nil { - t.Fatal(err) - } - - if f.Name != "testDeriveSubdomains.region1.example.com" { - t.Fatalf("unexpected function name '%v'", f.Name) - } -} - // TestNamed ensures that an explicitly passed name is used in leau of the // path derived name when provided, and persists through instantiations. func TestNamed(t *testing.T) { @@ -387,8 +351,8 @@ func TestDeriveImageDefaultRegistry(t *testing.T) { func TestCreateDelegates(t *testing.T) { var ( root = "testdata/example.com/testCreateDelegates" // .. in which to initialize - expectedName = "testCreateDelegates.example.com" // expected to be derived - expectedImage = "quay.io/alice/testCreateDelegates.example.com:latest" + expectedName = "testCreateDelegates" // expected to be derived + expectedImage = "quay.io/alice/testCreateDelegates:latest" builder = mock.NewBuilder() pusher = mock.NewPusher() deployer = mock.NewDeployer() @@ -501,8 +465,8 @@ func TestRun(t *testing.T) { func TestUpdate(t *testing.T) { var ( root = "testdata/example.com/testUpdate" - expectedName = "testUpdate.example.com" - expectedImage = "quay.io/alice/testUpdate.example.com:latest" + expectedName = "testUpdate" + expectedImage = "quay.io/alice/testUpdate:latest" builder = mock.NewBuilder() pusher = mock.NewPusher() updater = mock.NewUpdater() @@ -580,7 +544,7 @@ func TestUpdate(t *testing.T) { func TestRemoveByPath(t *testing.T) { var ( root = "testdata/example.com/testRemoveByPath" - expectedName = "testRemoveByPath.example.com" + expectedName = "testRemoveByPath" remover = mock.NewRemover() ) diff --git a/cmd/create.go b/cmd/create.go index a9a98ad71d..62add8f9a2 100644 --- a/cmd/create.go +++ b/cmd/create.go @@ -20,7 +20,6 @@ func init() { createCmd.Flags().BoolP("confirm", "c", false, "Prompt to confirm all configuration options - $FAAS_CONFIRM") createCmd.Flags().StringP("image", "i", "", "Optional full image name, in form [registry]/[namespace]/[name]:[tag] for example quay.io/myrepo/project.name:latest (overrides --repository) - $FAAS_IMAGE") createCmd.Flags().StringP("namespace", "n", "", "Override namespace into which the Function is deployed (on supported platforms). Default is to use currently active underlying platform setting - $FAAS_NAMESPACE") - createCmd.Flags().StringP("path", "p", cwd(), "Path to the new project directory - $FAAS_PATH") createCmd.Flags().StringP("repository", "r", "", "Repository for built images, ex 'docker.io/myuser' or just 'myuser'. Optional if --image provided. - $FAAS_REPOSITORY") createCmd.Flags().StringP("runtime", "l", faas.DefaultRuntime, "Function runtime language/framework. - $FAAS_RUNTIME") createCmd.Flags().StringP("templates", "", filepath.Join(configPath(), "templates"), "Extensible templates path. - $FAAS_TEMPLATES") @@ -38,10 +37,10 @@ func init() { } var createCmd = &cobra.Command{ - Use: "create ", + Use: "create ", Short: "Create a new Function, including initialization of local files and deployment.", SuggestFor: []string{"cerate", "new"}, - PreRunE: bindEnv("image", "namespace", "path", "repository", "runtime", "templates", "trigger", "confirm"), + PreRunE: bindEnv("image", "namespace", "repository", "runtime", "templates", "trigger", "confirm"), RunE: runCreate, } @@ -49,7 +48,7 @@ func runCreate(cmd *cobra.Command, args []string) (err error) { config := newCreateConfig(args).Prompt() function := faas.Function{ - Name: config.Name, + Name: config.initConfig.Name, Root: config.initConfig.Path, Runtime: config.initConfig.Runtime, Trigger: config.Trigger, @@ -106,23 +105,24 @@ func newCreateConfig(args []string) createConfig { } } -// Prompt the user with value of config members, allowing for interaractive changes. +// Prompt the user with value of config members, allowing for interactive changes. // Skipped if not in an interactive terminal (non-TTY), or if --confirm (agree to // all prompts) was not explicitly set. func (c createConfig) Prompt() createConfig { - name := deriveName(c.Name, c.initConfig.Path) if !interactiveTerminal() || !c.initConfig.Confirm { // Just print the basics if not confirming fmt.Printf("Project path: %v\n", c.initConfig.Path) - fmt.Printf("Project name: %v\n", name) + fmt.Printf("Function name: %v\n", c.initConfig.Name) fmt.Printf("Runtime: %v\n", c.Runtime) fmt.Printf("Trigger: %v\n", c.Trigger) return c } + + derivedName, derivedPath := deriveNameAndAbsolutePathFromPath(prompt.ForString("Project path", c.initConfig.Path, prompt.WithRequired(true))) return createConfig{ initConfig: initConfig{ - Path: prompt.ForString("Project path", c.initConfig.Path), - Name: prompt.ForString("Project name", name, prompt.WithRequired(true)), + Name: derivedName, + Path: derivedPath, Runtime: prompt.ForString("Runtime", c.Runtime), Trigger: prompt.ForString("Trigger", c.Trigger), // Templates intentionally omitted from prompt for being an edge case. diff --git a/cmd/init.go b/cmd/init.go index 2ff8a194d3..542672efbd 100644 --- a/cmd/init.go +++ b/cmd/init.go @@ -14,7 +14,6 @@ import ( func init() { root.AddCommand(initCmd) initCmd.Flags().BoolP("confirm", "c", false, "Prompt to confirm all configuration options - $FAAS_CONFIRM") - initCmd.Flags().StringP("path", "p", cwd(), "Path to the new project directory - $FAAS_PATH") initCmd.Flags().StringP("runtime", "l", faas.DefaultRuntime, "Function runtime language/framework. - $FAAS_RUNTIME") initCmd.Flags().StringP("templates", "", filepath.Join(configPath(), "templates"), "Extensible templates path. - $FAAS_TEMPLATES") initCmd.Flags().StringP("trigger", "t", faas.DefaultTrigger, "Function trigger (ex: 'http','events') - $FAAS_TRIGGER") @@ -25,10 +24,10 @@ func init() { } var initCmd = &cobra.Command{ - Use: "init ", + Use: "init ", Short: "Initialize a new Function project", SuggestFor: []string{"inti", "new"}, - PreRunE: bindEnv("path", "runtime", "templates", "trigger", "confirm"), + PreRunE: bindEnv("runtime", "templates", "trigger", "confirm"), RunE: runInit, // TODO: autocomplate Functions for runtime and trigger. } @@ -49,10 +48,10 @@ func runInit(cmd *cobra.Command, args []string) error { } type initConfig struct { - // Name of the service in DNS-compatible format (ex myfunc.example.com) + // Name of the Function. Name string - // Path to files on disk. Defaults to current working directory. + // Absolute path to Function on disk. Path string // Runtime language/framework. @@ -78,13 +77,15 @@ type initConfig struct { // newInitConfig returns a config populated from the current execution context // (args, flags and environment variables) func newInitConfig(args []string) initConfig { - var name string + var path string if len(args) > 0 { - name = args[0] // If explicitly provided, use. + path = args[0] // If explicitly provided, use. } + + derivedName, derivedPath := deriveNameAndAbsolutePathFromPath(path) return initConfig{ - Name: deriveName(name, viper.GetString("path")), // args[0] or derived - Path: viper.GetString("path"), + Name: derivedName, + Path: derivedPath, Runtime: viper.GetString("runtime"), Templates: viper.GetString("templates"), Trigger: viper.GetString("trigger"), @@ -96,19 +97,19 @@ func newInitConfig(args []string) initConfig { // Skipped if not in an interactive terminal (non-TTY), or if --confirm false (agree to // all prompts) was set (default). func (c initConfig) Prompt() initConfig { - name := deriveName(c.Name, c.Path) if !interactiveTerminal() || !c.Confirm { // Just print the basics if not confirming fmt.Printf("Project path: %v\n", c.Path) - fmt.Printf("Project name: %v\n", name) + fmt.Printf("Function name: %v\n", c.Name) fmt.Printf("Runtime: %v\n", c.Runtime) fmt.Printf("Trigger: %v\n", c.Trigger) return c } + + derivedName, derivedPath := deriveNameAndAbsolutePathFromPath(prompt.ForString("Project path", c.Path, prompt.WithRequired(true))) return initConfig{ - // TODO: Path should be prompted for and set prior to name attempting path derivation. Test/fix this if necessary. - Path: prompt.ForString("Project path", c.Path), - Name: prompt.ForString("Project name", name, prompt.WithRequired(true)), + Name: derivedName, + Path: derivedPath, Runtime: prompt.ForString("Runtime", c.Runtime), Trigger: prompt.ForString("Trigger", c.Trigger), // Templates intentiopnally omitted from prompt for being an edge case. diff --git a/cmd/root.go b/cmd/root.go index 9bc6ba77b8..890484332f 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -4,6 +4,7 @@ import ( "fmt" "os" "path/filepath" + "strings" "github.com/mitchellh/go-homedir" "github.com/ory/viper" @@ -165,21 +166,42 @@ func functionWithOverrides(root, namespace, image string) (f faas.Function, err // deriveName returns the explicit value (if provided) or attempts to derive // from the given path. Path is defaulted to current working directory, where -// a function configuration, if it exists and contains a name, is used. Lastly -// derivation using the path us used. +// a Function configuration, if it exists and contains a name, is used. func deriveName(explicitName string, path string) string { // If the name was explicitly provided, use it. if explicitName != "" { return explicitName } + // If the directory at path contains an initialized Function, use the name therein f, err := faas.NewFunction(path) if err == nil && f.Name != "" { return f.Name } - maxRecursion := faas.DefaultMaxRecursion - derivedName, _ := faas.DerivedName(path, maxRecursion) - return derivedName + + return "" +} + +// deriveNameAndAbsolutePathFromPath returns resolved Function name and absolute path +// to the Function project root. The input parameter path could be one of: +// 'relative/path/to/foo', '/absolute/path/to/foo', 'foo' or '' +func deriveNameAndAbsolutePathFromPath(path string) (string, string) { + var absPath string + + // If path is not specifed, we would like to use current working dir + if path == "" { + path = cwd() + } + + // Expand the passed Function name to its absolute path + absPath, err := filepath.Abs(path) + if err != nil { + return "", "" + } + + // Get the name of the Function, which equals to name of the current directory + pathParts := strings.Split(strings.TrimRight(path, string(os.PathSeparator)), string(os.PathSeparator)) + return pathParts[len(pathParts)-1], absPath } // deriveImage returns the same image name which will be used if no explicit diff --git a/docs/commands.md b/docs/commands.md index 6956c4deb8..f3736c6d5b 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -2,7 +2,7 @@ ## `init` -Creates a new Function project at _`path`_. If _`path`_ is unspecified, assumes the current directory. If _`path`_ does not exist, it will be created. The user can specify the runtime and trigger with flags. +Creates a new Function project at _`path`_. If _`path`_ is unspecified, assumes the current directory. If _`path`_ does not exist, it will be created. The function name is the name of the leaf directory at path. The user can specify the runtime and trigger with flags. Similar `kn` command: none. diff --git a/function.go b/function.go index 16e5ed225d..c393195819 100644 --- a/function.go +++ b/function.go @@ -7,8 +7,6 @@ import ( "os" "path/filepath" "strings" - - "golang.org/x/net/publicsuffix" ) type Function struct { @@ -49,6 +47,7 @@ type Function struct { // NewFunction creates a Function struct whose attributes are loaded from the // configuraiton located at path. func NewFunction(root string) (f Function, err error) { + // Expand the passed root to its absolute path (default current dir) if root, err = filepath.Abs(root); err != nil { return @@ -60,6 +59,12 @@ func NewFunction(root string) (f Function, err error) { return } + // Let's set Function name, if it is not already set + if c.Name == "" { + pathParts := strings.Split(strings.TrimRight(root, string(os.PathSeparator)), string(os.PathSeparator)) + c.Name = pathParts[len(pathParts)-1] + } + // set Function to the value of the config loaded from disk. f = fromConfig(c) @@ -143,16 +148,6 @@ func DerivedImage(root, repository string) (image string, err error) { return } -// DerivedName returns a name derived from the path, limited in its upward -// recursion along path to searchLimit. -func DerivedName(root string, searchLimit int) (string, error) { - root, err := filepath.Abs(root) - if err != nil { - return "", err - } - return pathToDomain(root, searchLimit), nil -} - // assertEmptyRoot ensures that the directory is empty enough to be used for // initializing a new Function. func assertEmptyRoot(path string) (err error) { @@ -208,83 +203,3 @@ func isEffectivelyEmpty(dir string) (bool, error) { } return true, nil } - -// Convert a path to a domain. -// Searches up the path string until a domain (TLD+1) is detected. -// Subdirectories are considered subdomains. -// Ex: Path: "/home/users/jane/src/example.com/admin/www" -// Returns: "www.admin.example.com" -// maxLevels is the number of directories to walk upwards beyond the current -// directory to determine domain (i.e. current directory is always considered. -// Zero indicates only consider last path element.) -func pathToDomain(path string, maxLevels int) string { - var ( - // parts of the path, separated by os separator - parts = strings.Split(path, string(os.PathSeparator)) - - // subdomains derived from the path - subdomains []string - - // domain derived from the path - domain string - ) - - // Loop over parts from back to front (recursing upwards), building - // optional subdomains until a root domain (TLD+1) is detected. - for i := len(parts) - 1; i >= 0; i-- { - part := parts[i] - - // Support limited recursion - // Tests, for instance, need to be allowed to reliably fail by having their - // recursion contained within ./testdata if recursion is set to -1, there - // is no limit. 0 indicates only the current directory is considered. - iteration := len(parts) - 1 - i - if maxLevels >= 0 && iteration > maxLevels { - break - } - - // Detect TLD+1 - // If the current directory has a valid TLD plus one, it is a match. - // This is determined by using the public suffices list, which includes - // both ICANN managed TLDs as well as an extended list (matching, for - // instance 'cluster.local') - if suffix, _ := publicsuffix.EffectiveTLDPlusOne(part); suffix != "" { - domain = part - break // no directories above the nearest TLD+1 should be considered. - } - - // Skip blanks - // Path elements which are blank, such as in the case of a trailing slash - // are ignored and the recursion continues, effectively collapsing ex: '//'. - if part == "" { - continue - } - - // Build subdomain - // Each path element which appears before the TLD+1 is a subdomain. - // ex: '/home/users/jane/src/example.com/us-west-2/admin/www' creates the - // subdomain []string{'www', 'admin', 'us-west-2'} - subdomains = append(subdomains, part) - } - - // Unable to derive domain - // If the entire path was searched, but no parts matched a TLD+1, the domain - // will be blank. In this case, the path was insufficient to derive a domain - // ex "/home/users/jane/src/test" contains no TLD, thus the final domain must - // be explicitly provided. - if domain == "" { - return "" - } - - // Prepend subdomains - // If the path was a subdirectory within a TLD+1, these sudbomains - // are prepended to the TLD+1 to create the final domain. - // ex: '/home/users/jane/src/example.com/us-west-2/admin/www' yields - // www.admin.use-west-2.example.com - if len(subdomains) > 0 { - subdomains = append(subdomains, domain) - return strings.Join(subdomains, ".") - } - - return domain -} diff --git a/function_unit_test.go b/function_unit_test.go deleted file mode 100644 index 1b62ca8e39..0000000000 --- a/function_unit_test.go +++ /dev/null @@ -1,76 +0,0 @@ -package faas - -import ( - "path/filepath" - "testing" -) - -// TestPathToDomain validatest the calculation used to derive a default domain -// for a Function from the current directory filepath (used as a default -// in the absence of an explicit setting). -// NOTE: although the implementation uses os.PathSeparator and is developed with -// non-unix platforms in mind, these tests are written using unix-style paths. -func TestPathToDomain(t *testing.T) { - var noLimit = -1 - tests := []struct { - Path string // input filepath - Limit int // upward recursion limit - Domain string // Expected returned domain value - }{ - // empty input should not find a domain. - {filepath.Join(""), noLimit, ""}, - - // trailing slashes ignored - {filepath.Join("home", "user", "example.com", "admin"), noLimit, "admin.example.com"}, - - // root filepath should not find a domain. - {filepath.Join(""), noLimit, ""}, - - // valid filepath but without a domain should find no domain. - {filepath.Join("home","user"), noLimit, ""}, - - // valid domain as the current directory should be found. - {filepath.Join("home","user","example.com"), noLimit, "example.com"}, - - // valid domain as the current directory should be found even with recursion disabled. - {filepath.Join("home", "user", "example.com"), 0, "example.com"}, - - // Subdomain - {filepath.Join("src", "example.com", "www"), noLimit, "www.example.com"}, - - // Subdomain with recursion disabled should not find the domain. - {filepath.Join("src", "example.com", "www"), 0, ""}, - - // Sub-subdomain - {filepath.Join("src", "example.com", "www", "test1"), noLimit, "test1.www.example.com"}, - - // Sub-subdomain with exact recursion limit to catch the domain - {filepath.Join("src", "example.com", "www", "test1"), 2, "test1.www.example.com"}, - - // CWD a valid TLD+1 (not suggested, but supported) - {filepath.Join("src", "my.example.com"), noLimit, "my.example.com"}, - - // Multi-level TLDs - {filepath.Join("src", "example.co.uk"), noLimit, "example.co.uk"}, - - // Multi-level TLDs with sub - {filepath.Join("src", "example.co.uk", "www"), noLimit, "www.example.co.uk"}, - - // Expected behavior is `test1.my.example.com` but will yield `test1.my` - // because .my is a TLD hence the reason why dots in directories to denote - // multiple levels of subdomain are technically supported but not - // recommended: unexpected behavior because the public suffices list is - // shifty. - {filepath.Join("src", "example.com", "test1.my"), noLimit, "test1.my"}, - - // Ensure that cluster.local is explicitly allowed. - {filepath.Join("src", "cluster.local", "www"), noLimit, "www.cluster.local"}, - } - - for _, test := range tests { - domain := pathToDomain(test.Path, test.Limit) - if domain != test.Domain { - t.Fatalf("expected filepath '%v' (limit %v) to yield domain '%v', got '%v'", test.Path, test.Limit, test.Domain, domain) - } - } -} diff --git a/k8s/names.go b/k8s/names.go index 3bf4cf7ee6..879b2b9407 100644 --- a/k8s/names.go +++ b/k8s/names.go @@ -7,12 +7,13 @@ import ( "k8s.io/apimachinery/pkg/util/validation" ) -// ToSubdomain converts a domain to a subdomain. -// If the input is not a valid domain an error is thrown. -func ToSubdomain(in string) (string, error) { - if err := validation.IsFullyQualifiedDomainName(nil, in); err != nil { - return "", err.ToAggregate() - } +// ToK8sAllowedName converts a name to a name that's allowed for k8s service. +// k8s does not support service names with dots. So encode it such that +// www.my-domain,com -> www-my--domain-com +// Input errors if not a 1035 label. +// "a DNS-1035 label must consist of lower case alphanumeric characters or '-', +// start with an alphabetic character, and end with an alphanumeric character" +func ToK8sAllowedName(in string) (string, error) { out := []rune{} for _, c := range in { @@ -26,13 +27,22 @@ func ToSubdomain(in string) (string, error) { out = append(out, c) } } - return string(out), nil + + result := string(out) + + if errs := validation.IsDNS1035Label(result); len(errs) > 0 { + return "", errors.New(strings.Join(errs, ",")) + } + + return result, nil } -// FromSubdomain converts a doman which has been encoded as -// a subdomain using the algorithm of ToSubdoman back to a domain. +// FromK8sAllowedName converts a name which has been encoded as +// an allowed k8s name using the algorithm of ToK8sAllowedName back to the original. +// www-my--domain-com -> www.my-domain.com // Input errors if not a 1035 label. -func FromSubdomain(in string) (string, error) { +func FromK8sAllowedName(in string) (string, error) { + if errs := validation.IsDNS1035Label(in); len(errs) > 0 { return "", errors.New(strings.Join(errs, ",")) } diff --git a/k8s/names_test.go b/k8s/names_test.go index 8262f78c47..5bce7cdd9c 100644 --- a/k8s/names_test.go +++ b/k8s/names_test.go @@ -2,25 +2,25 @@ package k8s import "testing" -// TestToSubdomain ensures that a valid domain name is -// encoded into the expected subdmain. -func TestToSubdomain(t *testing.T) { +// TestToK8sAllowedName ensures that a valid name is +// encoded into k8s allowed name. +func TestToK8sAllowedName(t *testing.T) { cases := []struct { In string Out string Err bool }{ - {"", "", true}, // invalid domain - {"*", "", true}, // invalid domain - {"example", "", true}, // invalid domain + {"", "", true}, // invalid name + {"*", "", true}, // invalid name + {"example", "example", true}, {"example.com", "example-com", false}, {"my-domain.com", "my--domain-com", false}, } for _, c := range cases { - out, err := ToSubdomain(c.In) + out, err := ToK8sAllowedName(c.In) if err != nil && !c.Err { - t.Fatalf("Unexpected error: %v", err) + t.Fatalf("Unexpected error: %v, for '%v', want '%v', but got '%v'", err, c.In, c.Out, out) } if out != c.Out { t.Fatalf("expected '%v' to yield '%v', got '%v'", c.In, c.Out, out) @@ -28,9 +28,9 @@ func TestToSubdomain(t *testing.T) { } } -// TestFromSubdomain ensures that a valid subdomain is decoded -// back into a domain. -func TestFromSubdomain(t *testing.T) { +// TestFromK8sAllowedName ensures that an allowed k8s name is correctly +// decoded back into the original name. +func TestFromK8sAllowedName(t *testing.T) { cases := []struct { In string Out string @@ -44,7 +44,7 @@ func TestFromSubdomain(t *testing.T) { } for _, c := range cases { - out, err := FromSubdomain(c.In) + out, err := FromK8sAllowedName(c.In) if err != nil && !c.Err { t.Fatalf("Unexpected error: %v", err) } diff --git a/knative/deployer.go b/knative/deployer.go index 5ef00d2c84..994dbabb2c 100644 --- a/knative/deployer.go +++ b/knative/deployer.go @@ -5,7 +5,6 @@ import ( "fmt" "io" "os" - "strings" commands "knative.dev/client/pkg/kn/commands" "knative.dev/client/pkg/kn/core" @@ -33,7 +32,7 @@ func NewDeployer() *Deployer { func (d *Deployer) Deploy(f faas.Function) (err error) { // k8s does not support service names with dots. so encode it such that // www.my-domain,com -> www-my--domain-com - encodedName, err := k8s.ToSubdomain(f.Name) + encodedName, err := k8s.ToK8sAllowedName(f.Name) if err != nil { return } @@ -46,22 +45,6 @@ func (d *Deployer) Deploy(f faas.Function) (err error) { output = &bytes.Buffer{} } - // FIXME(lkinglan): The labels set explicitly here may interfere with the - // cluster configuraiton steps described in the documentation, and may also - // break on multi-level subdomains or if they are out of sync with that - // configuration. These could be removed from here, and instead the cluster - // expeted to be configured correctly. It is a future enhancement that an - // attempt to deploy a publicly accessible Function of a hithertoo unseen - // TLD+1 will modify this config-map. - // See https://github.com/boson-project/faas/issues/47 - nn := strings.Split(f.Name, ".") - if len(nn) < 3 { - err = fmt.Errorf("invalid service name '%v', must be at least three parts.\n", f.Name) - return - } - subDomain := nn[0] - domain := strings.Join(nn[1:], ".") - params := commands.KnParams{} params.Initialize() params.Output = output @@ -71,8 +54,6 @@ func (d *Deployer) Deploy(f faas.Function) (err error) { "service", "create", encodedName, "--image", f.Image, "--env", "VERBOSE=true", - "--label", fmt.Sprintf("faas.domain=%s", domain), - "--annotation", fmt.Sprintf("faas.subdomain=%s", subDomain), "--label", "bosonFunction=true", } if d.Namespace != "" { diff --git a/knative/describer.go b/knative/describer.go index 9fcd4d5214..de3e536978 100644 --- a/knative/describer.go +++ b/knative/describer.go @@ -52,7 +52,7 @@ func (describer *Describer) Describe(name string) (description faas.Description, servingClient := describer.servingClient eventingClient := describer.eventingClient - serviceName, err := k8s.ToSubdomain(name) + serviceName, err := k8s.ToK8sAllowedName(name) if err != nil { return } @@ -101,7 +101,7 @@ func (describer *Describer) Describe(name string) (description faas.Description, description.Routes = routeURLs description.Subscriptions = subscriptions - description.Name, err = k8s.FromSubdomain(service.Name) + description.Name, err = k8s.FromK8sAllowedName(service.Name) return } diff --git a/knative/lister.go b/knative/lister.go index a1eee5e151..28df3db54d 100644 --- a/knative/lister.go +++ b/knative/lister.go @@ -38,7 +38,7 @@ func (l *Lister) List() (names []string, err error) { for _, service := range lst.Items { // Convert the "subdomain-encoded" (i.e. kube-service-friendly) name // back out to a fully qualified service name. - n, err := k8s.FromSubdomain(service.Name) + n, err := k8s.FromK8sAllowedName(service.Name) if err != nil { return names, err } diff --git a/knative/remover.go b/knative/remover.go index e4fa77c412..5e4094c321 100644 --- a/knative/remover.go +++ b/knative/remover.go @@ -22,7 +22,7 @@ type Remover struct { func (remover *Remover) Remove(name string) (err error) { - project, err := k8s.ToSubdomain(name) + project, err := k8s.ToK8sAllowedName(name) if err != nil { return } diff --git a/knative/updater.go b/knative/updater.go index c7bf53f30b..ad2cea04ec 100644 --- a/knative/updater.go +++ b/knative/updater.go @@ -38,9 +38,9 @@ func NewUpdater(namespaceOverride string) (updater *Updater, err error) { func (updater *Updater) Update(f faas.Function) error { client, namespace := updater.client, updater.namespace - project, err := k8s.ToSubdomain(f.Name) + project, err := k8s.ToK8sAllowedName(f.Name) if err != nil { - return fmt.Errorf("updater call to k8s.ToSubdomain failed: %v", err) + return fmt.Errorf("updater call to k8s.ToK8sAllowedName failed: %v", err) } service, err := client.Services(namespace).Get(project, apiMachineryV1.GetOptions{})