Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,14 @@ package cmd

import (
"errors"
"fmt"
"os"
"path/filepath"
"time"

log "github.com/sirupsen/logrus"
"github.com/spf13/cobra"
"github.com/srl-labs/containerlab/utils"
)

var (
Expand Down Expand Up @@ -106,12 +108,27 @@ func getTopoFilePath(cmd *cobra.Command) error {
return nil
}

var err error
if utils.IsHttpUri(topo){
switch {
case utils.IsGitHubURL(topo):
err := utils.CheckSuffix(topo)
if err != nil {
return err
}
rawUrl := utils.GetRawURL(topo)
topo = utils.FilenameForURL(topo)
utils.CopyFileContents(rawUrl, topo, 0754)
default:
return fmt.Errorf("unsupported git repositoy: %s", topo)
}
}

// if topo or name flags have been provided, don't try to derive the topo file
if topo != "" || name != "" {
return nil
}

var err error

log.Debugf("trying to find topology files automatically")

Expand Down
30 changes: 30 additions & 0 deletions utils/http.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package utils

import (
"strings"
"errors"
)


func GetRawURL(url string) string {
if strings.Contains(url, "github.com") {
raw:=strings.Replace(url, "github.com", "raw.githubusercontent.com", 1)
return strings.Replace(raw, "/blob", "", 1)
}
return url
}

func IsGitHubURL(url string) bool {
return strings.Contains(url, "github")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This contains in my view is too broad. since also a "raw.githubusercontent.com" url would match.
In cmd/root.go you use this in the switch and consecutively apply the GetRawUrl. Which then does the conversion.
although that would check for "github.com" it should probably be more kind of "consistent".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I'm grasping what you'd like to see from this method.
You mentioned that it would match on raw.githubusercontent.com and you're right. I had planned on it matching on either a generic github.com or the raw.github path to offer flexibility to the end user. It then proceeds to that GetRawURL method where it only converts it if its a generic github.com url else it returns the url without converting anything.
How would you envision this changing?


}

// required global variable for tests, otherwise comparison operator fails as error instances were not equal
var ErrInvalidSuffix = errors.New("valid URL passed in as topology file, but does not end with .yml or .yaml, endpoint must be an actual topology file")
func CheckSuffix(url string) error {
// check if topo ends with either .yml or .yaml
if !strings.HasSuffix(url, ".yml") && !strings.HasSuffix(url, ".yaml") {
return ErrInvalidSuffix
}
return nil
}
67 changes: 67 additions & 0 deletions utils/http_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
package utils

import (
"testing"
)

func TestIsGithubURL(t *testing.T) {
// tests that github urls are detected
var tests = []struct {
input string
expected bool
}{
{"github.com", true},
{"github.com/containers/containerlab/blob/master/README.md", true},
{"google.com/containers", false},
{"google.com/containers/containerlab/blob/master/README.md", false},
{"gitlab.com/containers", false},
{"raw.githubusercontent.com/containers", true},
}
for _, test := range tests {
if output := IsGitHubURL(test.input); output != test.expected {
t.Error("Test Failed: {} inputted, {} expected, recieved: {}", test.input, test.expected, output)
}
}
}

func TestGetRawURL(t *testing.T) {
// tests that github urls are converted to raw urls evething else is left as is
var tests = []struct {
input string
expected string
}{
{"github.com", "raw.githubusercontent.com"},
{"github.com/containers/containerlab/blob/master/README.md", "raw.githubusercontent.com/containers/containerlab/master/README.md"},
{"google.com/containers", "google.com/containers"},
{"google.com/containers/containerlab/blob/master/README.md", "google.com/containers/containerlab/blob/master/README.md"},
{"gitlab.com/containers", "gitlab.com/containers"},
{"raw.githubusercontent.com/containers", "raw.githubusercontent.com/containers"},
}
for _, test := range tests {
if output := GetRawURL(test.input); output != test.expected {
t.Error("Test Failed: {} inputted, {} expected, recieved: {}", test.input, test.expected, output)
}
}
}

func TestCheckSuffix(t *testing.T) {
// tests for valid suffix
var tests = []struct {
input string
expected error
}{
{"github.com", ErrInvalidSuffix},
{"github.com/containers/containerlab/blob/master/README.md", ErrInvalidSuffix},
{"google.com/containers", ErrInvalidSuffix},
{"google.com/containers/containerlab/blob/master/README.md", ErrInvalidSuffix},
{"gitlab.com/containers", ErrInvalidSuffix},
{"raw.githubusercontent.com/containers", ErrInvalidSuffix},
{"github.com/containers/containerlab/blob/master/README.yml", nil},
{"github.com/containers/containerlab/blob/master/README.yaml", nil},
}
for _, test := range tests {
if output := CheckSuffix(test.input); output != test.expected {
t.Error("Test Failed: {} inputted, {} expected, recieved: {}", test.input, test.expected, output)
}
}
}