Git Product home page Git Product logo

go-vcloud-director's Introduction

go-vcloud-director GoDoc Chat

This repo contains the go-vcloud-director package which implements an SDK for VMware Cloud Director. The project serves the needs of Golang developers who need to integrate with VMware Cloud Director. It is also the basis of the vCD Terraform Provider.

Contributions

Contributions to go-vcloud-director are gladly welcome and range from participating in community discussions to submitting pull requests. Please see the contributing guide for details on joining the community.

Install and Build

This project started using Go modules starting with version 2.1 and vendor directory is no longer bundled.

As per the modules documentation you no longer need to use GOPATH. You can clone the branch in any directory you like and go will fetch dependencies specified in the go.mod file:

cd ~/Documents/mycode
git clone https://github.com/vmware/go-vcloud-director.git
cd go-vcloud-director/govcd
go build

Note Do not forget to set GO111MODULE=on if you are in your GOPATH. Read more about this.

Example code

To show the SDK in action run the example:

mkdir ~/govcd_example
go mod init govcd_example
go get github.com/vmware/go-vcloud-director/v2@main
go build -o example
./example user_name "password" org_name vcd_IP vdc_name 

Here's the code:

package main

import (
	"fmt"
	"net/url"
	"os"

	"github.com/vmware/go-vcloud-director/v2/govcd"
)

type Config struct {
	User     string
	Password string
	Org      string
	Href     string
	VDC      string
	Insecure bool
}

func (c *Config) Client() (*govcd.VCDClient, error) {
	u, err := url.ParseRequestURI(c.Href)
	if err != nil {
		return nil, fmt.Errorf("unable to pass url: %s", err)
	}

	vcdclient := govcd.NewVCDClient(*u, c.Insecure)
	err = vcdclient.Authenticate(c.User, c.Password, c.Org)
	if err != nil {
		return nil, fmt.Errorf("unable to authenticate: %s", err)
	}
	return vcdclient, nil
}

func main() {
	if len(os.Args) < 6 {
		fmt.Println("Syntax: example user password org VCD_IP VDC ")
		os.Exit(1)
	}
	config := Config{
		User:     os.Args[1],
		Password: os.Args[2],
		Org:      os.Args[3],
		Href:     fmt.Sprintf("https://%s/api", os.Args[4]),
		VDC:      os.Args[5],
		Insecure: true,
	}

	client, err := config.Client() // We now have a client
	if err != nil {
		fmt.Println(err)
		os.Exit(1)
	}
	org, err := client.GetOrgByName(config.Org)
	if err != nil {
		fmt.Println(err)
		os.Exit(1)
	}
	vdc, err := org.GetVDCByName(config.VDC, false)
	if err != nil {
		fmt.Println(err)
		os.Exit(1)
	}
	fmt.Printf("Org URL: %s\n", org.Org.HREF)
	fmt.Printf("VDC URL: %s\n", vdc.Vdc.HREF)
}

Authentication

You can authenticate to the vCD in five ways:

  • With a System Administration user and password (administrator@system)
  • With an Organization user and password (tenant-admin@org-name)

For the above two methods, you use:

	err := vcdClient.Authenticate(User, Password, Org)
	// or
	resp, err := vcdClient.GetAuthResponse(User, Password, Org)
  • With an authorization token
	err := vcdClient.SetToken(Org, govcd.AuthorizationHeader, Token)

The file scripts/get_token.sh provides a handy method of extracting the token (x-vcloud-authorization value) for future use.

  • With a service account token (the file needs to have r+w rights)
	err := vcdClient.SetServiceAccountApiToken(Org, "tokenfile.json")
  • SAML user and password (works with ADFS as IdP using WS-TRUST endpoint "/adfs/services/trust/13/usernamemixed"). One must pass govcd.WithSamlAdfs(true,customAdfsRptId) and username must be formatted so that ADFS understands it ('[email protected]' or 'contoso.com\user') You can find usage example in samples/saml_auth_adfs.
vcdCli := govcd.NewVCDClient(*vcdURL, true, govcd.WithSamlAdfs(true, customAdfsRptId))
err = vcdCli.Authenticate(username, password, org)

More information about inner workings of SAML auth flow in this codebase can be found in saml_auth.go:authorizeSamlAdfs(...). Additionaly this flow is documented in vCD documentation.

go-vcloud-director's People

Contributors

adambarreiro avatar adezxc avatar auppunda avatar bobbydeveaux avatar casualjim avatar crashiura avatar dataclouder avatar devopsbrett avatar didainius avatar dn1s avatar frapposelli avatar frenchtoasters avatar gitter-badger avatar harmjanblok avatar higebu avatar justnisar avatar lvirbalas avatar nickithewatt avatar pasikarkkainen avatar rickard-von-essen avatar robcoward avatar slargo avatar slavaavr avatar superseb avatar thomsmoreau avatar tlawrence avatar ty2 avatar vbauzys avatar vmanasiev avatar yusufozturk avatar

Stargazers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

Watchers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

go-vcloud-director's Issues

Reference a need for local testing before doing a Pull Request

Currently, if you read CONTRIBUTING.md you get the following message:

1.) "Fork the Repo" <...>
2.) "Make Changes and Commit" <...>
3.) "Create a Pull Request"

We need a step explaining that the tests should be executed locally before requesting a PR.

Add VApp Lease settings to OrgSettings

I am not sure if there is a bug on the vcd or the documentation for the rest api isn't a 100% accurate, but I ran into some issues with updating orgs when I added VApp settings into the struct OrgSettings. However, we eventually want the user to be able to set those settings. This bug doesn't stop us from having crud operations for Orgs.

Fix README.md to reflect new project state

The README.md reflects the old govcloudair documentation. Needs to be updated to contain a good intro to the project including succinct description of how to build as well as pointers to other docs like CONTRIBUTING.md.

Improve testing config file to be more self-describable

Context

Currently it's easy to put incorrect values into the config.yaml file, because it's not clear how the tests work. When you see the config for the first time, you think - will the tests create these resources or is it asking for existing ones?

Another issue is that the user needs to create this file manually, also its name is too generic (config.yaml).

Requests

To make it easier for the contributors to run tests:

1.) Rename the config.yaml -> govcd_test_config.yaml
2.) Create a sample file in the repository (currently, the user needs to create it from scratch).
3.) Add in-line comments to the file describing each field that requires an explanation. Luckily, the Yaml parser we use recognises comments.
4.) Update TESTING.md accordingly.

Specific Examples

Below is a list of specific examples, where a reasonable comment would resolve the confusion.

  • "user:" - it's not clear whether it's asking for System Administrator, which will create everything else, or an existing Org Administrator.
  • "org:", "vdc:", "catalog:" items, "vapp:", "network:", "edgetogateway:" - existing ones or names for the new ones that will get created?
  • "externalip:" - an unused, but available external IP, or an IP that the Edge Gateway has (currently, it's the latter). If you make a mistake here, you'll get the following error:
"error reconfiguring Edge Gateway: API Error: 400: [ d212c6c4-51b0-496e-b232-0164780300b3 ] External IP 10.150.211.54-10.150.211.54 for NAT should belong to the sub-allocated IP range  of the edge gateway."
  • "internalip:" - again, not clear for what purpose and whether it's an assigned one or free.

We need a comment above or besides each of the above parameter lines in the config file, explaining how to specify them without guesswork.

logging of http request may empty input data

The logging of HTTP requests in api.go may empty the data when the body is not a *bytes.Buffer.

Most of the requests in the API are created using a *bytes.Buffer, but some aren't. When the body is some other kind of io.Reader, copying the contents will empty the buffer, and the request will fail.

To fix it, we will check whether the body type is *bytes.Buffer and make the copy only in that case.

Org CRUD implementation

Create, Read, Update, Delete for Organization needs to be eventually pushed into the repo

Add Vendors to the project to simplify and fixate dependencies

Context

There are two groups of developers of our project:
a.) Developers, who just build the library (say, for Terraform).
b.) Contributors, who build & test the library.

Issue No. 1

If you're working from the main repo, pull in the code with go get, which pulls the dependencies automatically and builds just fine.
However, if you're contributing and pulling your fork, it won't work due to the default path being not under /vmware. In this case, you'll have to clone manually. But if you git clone the tests fail with the following errors and you'll have to go get the packages manually:

$ go test -check.v .
# github.com/vmware/go-vcloud-director/govcd
api_vcd_test.go:6:2: cannot find package "gopkg.in/check.v1" in any of:
	/usr/local/go/src/gopkg.in/check.v1 (from $GOROOT)
	/Users/lvirbalas/go/src/gopkg.in/check.v1 (from $GOPATH)
FAIL	github.com/vmware/go-vcloud-director/govcd [setup failed]

$ go get gopkg.in/check.v1

$ go test -check.v .
# github.com/vmware/go-vcloud-director/govcd
api_vcd_test.go:7:2: cannot find package "gopkg.in/yaml.v2" in any of:
	/usr/local/go/src/gopkg.in/yaml.v2 (from $GOROOT)
	/Users/lvirbalas/go/src/gopkg.in/yaml.v2 (from $GOPATH)
FAIL	github.com/vmware/go-vcloud-director/govcd [setup failed]

$ go get gopkg.in/yaml.v2

Issue No. 2

If dependent packages change, it may break the library.

Solution

Let's introduce the vendoring to the go-vcloud-director to address both.

Catalog CRUD implementation

Need Create, Read, Update, Delete for catalogs. This will follow the format of the org crud as we want to keep implementations consistent across the project

Make all DEBUG output in the library consistent

There are many calls that output a "DEBUG XML" line followed by the XML of the entity being processed.
In some cases, the output is conditional, depending on the value of an environment variable GOVCLOUDAIR_DEBUG.

We want to make this behavior predictable and consistent across all modules, as follows:

  • Every debug message will depend on a global bool variable named Vcd_debug;
  • The global variable will be initialized in the init function of api.go;
  • The Vcd_debug variable will be modified by the environment variable GOVCLOUDAIR_DEBUG (any non-empty value will enable the feature). Clients can override this setting with their own criteria, since the variable will be public.
  • Every current occurrence of log.Printf(format_string, variable) will be replaced by Debug_msg(format_string, variable), where Debug_msg is a function in api.go defined as follows:
func Debug_msg(format_message string, value interface{}) {
    if Vcd_debug {
        log.Printf(format_message, value)
    }
}

Convert hard-coded messages into global constants.

There are more than 1,000 hard-coded strings in the library, many of which are repeated several times: error messages, type identifiers, debug headers, REST URL components.)
Using them as they are is error prone, and it is not facilitated by editors and IDEs, in addition to duplicating lots of literal values unnecessarily.

We should convert the immutable text into global constants, to be used consistently across the library.

Timeout for long operations

Example: Triggered by running govcd tests

SIGQUIT: quit
PC=0x105be73 m=0 sigcode=0

goroutine 0 [idle]:
runtime.kevent(0x7fff00000004, 0x0, 0x0, 0x7fff5fbff0c0, 0x40, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
	/usr/local/Cellar/go/1.10.3/libexec/src/runtime/sys_darwin_amd64.s:621 +0x23
runtime.netpoll(0xc420034f01, 0xc420032a01)
	/usr/local/Cellar/go/1.10.3/libexec/src/runtime/netpoll_kqueue.go:79 +0x14c
runtime.findrunnable(0xc420030500, 0x0)
	/usr/local/Cellar/go/1.10.3/libexec/src/runtime/proc.go:2397 +0x65b
runtime.schedule()
	/usr/local/Cellar/go/1.10.3/libexec/src/runtime/proc.go:2541 +0x13b
runtime.park_m(0xc420134a80)
	/usr/local/Cellar/go/1.10.3/libexec/src/runtime/proc.go:2604 +0xb6
runtime.mcall(0x0)
	/usr/local/Cellar/go/1.10.3/libexec/src/runtime/asm_amd64.s:351 +0x5b

goroutine 1 [chan receive, 9 minutes]:
testing.(*T).Run(0xc42012a0f0, 0x1446d7e, 0x4, 0x14614f0, 0x1077bb6)
	/usr/local/Cellar/go/1.10.3/libexec/src/testing/testing.go:825 +0x301
testing.runTests.func1(0xc42012a000)
	/usr/local/Cellar/go/1.10.3/libexec/src/testing/testing.go:1063 +0x64
testing.tRunner(0xc42012a000, 0xc420065df8)
	/usr/local/Cellar/go/1.10.3/libexec/src/testing/testing.go:777 +0xd0
testing.runTests(0xc42010c500, 0x166c940, 0x3, 0x3, 0x10122b9)
	/usr/local/Cellar/go/1.10.3/libexec/src/testing/testing.go:1061 +0x2c4
testing.(*M).Run(0xc420126080, 0x0)
	/usr/local/Cellar/go/1.10.3/libexec/src/testing/testing.go:978 +0x171
main.main()
	_testmain.go:46 +0x151

goroutine 5 [chan receive, 3 minutes]:
github.com/vmware/go-vcloud-director/vendor/gopkg.in/check%2ev1.(*suiteRunner).runTest(0xc420126100, 0xc420132230, 0xc42012a1e0)
	/Users/Uppundaa/go2/src/github.com/vmware/go-vcloud-director/vendor/gopkg.in/check.v1/check.go:819 +0x55
github.com/vmware/go-vcloud-director/vendor/gopkg.in/check%2ev1.(*suiteRunner).run(0xc420126100, 0xc4200206e0)
	/Users/Uppundaa/go2/src/github.com/vmware/go-vcloud-director/vendor/gopkg.in/check.v1/check.go:624 +0xf2
github.com/vmware/go-vcloud-director/vendor/gopkg.in/check%2ev1.Run(0x143f320, 0xc4200206e0, 0xc420070cc0, 0x0)
	/Users/Uppundaa/go2/src/github.com/vmware/go-vcloud-director/vendor/gopkg.in/check.v1/run.go:92 +0x4d
github.com/vmware/go-vcloud-director/vendor/gopkg.in/check%2ev1.RunAll(0xc420070cc0, 0x0)
	/Users/Uppundaa/go2/src/github.com/vmware/go-vcloud-director/vendor/gopkg.in/check.v1/run.go:84 +0xa1
github.com/vmware/go-vcloud-director/vendor/gopkg.in/check%2ev1.TestingT(0xc42012a0f0)
	/Users/Uppundaa/go2/src/github.com/vmware/go-vcloud-director/vendor/gopkg.in/check.v1/run.go:72 +0x368
github.com/vmware/go-vcloud-director/govcd.Test(0xc42012a0f0)
	/Users/Uppundaa/go2/src/github.com/vmware/go-vcloud-director/govcd/api_vcd_test.go:91 +0x2b
testing.tRunner(0xc42012a0f0, 0x14614f0)
	/usr/local/Cellar/go/1.10.3/libexec/src/testing/testing.go:777 +0xd0
created by testing.(*T).Run
	/usr/local/Cellar/go/1.10.3/libexec/src/testing/testing.go:824 +0x2e0

goroutine 6 [select, 3 minutes]:
github.com/vmware/go-vcloud-director/vendor/gopkg.in/check%2ev1.(*resultTracker)._loopRoutine(0xc42012c120)
	/Users/Uppundaa/go2/src/github.com/vmware/go-vcloud-director/vendor/gopkg.in/check.v1/check.go:470 +0xf5
created by github.com/vmware/go-vcloud-director/vendor/gopkg.in/check%2ev1.(*resultTracker).start
	/Users/Uppundaa/go2/src/github.com/vmware/go-vcloud-director/vendor/gopkg.in/check.v1/check.go:450 +0x3f

goroutine 30 [IO wait]:
internal/poll.runtime_pollWait(0x1b00ea0, 0x72, 0xc42019d760)
	/usr/local/Cellar/go/1.10.3/libexec/src/runtime/netpoll.go:173 +0x57
internal/poll.(*pollDesc).wait(0xc420126818, 0x72, 0xffffffffffffff00, 0x1494460, 0x1639418)
	/usr/local/Cellar/go/1.10.3/libexec/src/internal/poll/fd_poll_runtime.go:85 +0x9b
internal/poll.(*pollDesc).waitRead(0xc420126818, 0xc42046a000, 0x4000, 0x4000)
	/usr/local/Cellar/go/1.10.3/libexec/src/internal/poll/fd_poll_runtime.go:90 +0x3d
internal/poll.(*FD).Read(0xc420126800, 0xc42046a000, 0x4000, 0x4000, 0x0, 0x0, 0x0)
	/usr/local/Cellar/go/1.10.3/libexec/src/internal/poll/fd_unix.go:157 +0x1dc
net.(*netFD).Read(0xc420126800, 0xc42046a000, 0x4000, 0x4000, 0x2d3, 0xc4202c2f70, 0x3)
	/usr/local/Cellar/go/1.10.3/libexec/src/net/fd_unix.go:202 +0x4f
net.(*conn).Read(0xc42000e298, 0xc42046a000, 0x4000, 0x4000, 0x0, 0x0, 0x0)
	/usr/local/Cellar/go/1.10.3/libexec/src/net/net.go:176 +0x6a
crypto/tls.(*block).readFromUntil(0xc4202be420, 0x1b00f70, 0xc42000e298, 0x5, 0xc42000e298, 0xc42019da90)
	/usr/local/Cellar/go/1.10.3/libexec/src/crypto/tls/conn.go:493 +0x96
crypto/tls.(*Conn).readRecord(0xc42009bc00, 0x1461d17, 0xc42009bd20, 0x1056b60)
	/usr/local/Cellar/go/1.10.3/libexec/src/crypto/tls/conn.go:595 +0xe0
crypto/tls.(*Conn).Read(0xc42009bc00, 0xc4201bb000, 0x1000, 0x1000, 0x0, 0x0, 0x0)
	/usr/local/Cellar/go/1.10.3/libexec/src/crypto/tls/conn.go:1156 +0x100
net/http.(*persistConn).Read(0xc4200cb320, 0xc4201bb000, 0x1000, 0x1000, 0xc42019db98, 0x10055b5, 0xc4202e4000)
	/usr/local/Cellar/go/1.10.3/libexec/src/net/http/transport.go:1453 +0x136
bufio.(*Reader).fill(0xc4202d4480)
	/usr/local/Cellar/go/1.10.3/libexec/src/bufio/bufio.go:100 +0x11e
bufio.(*Reader).Peek(0xc4202d4480, 0x1, 0x0, 0x0, 0x1, 0xc4203644e0, 0x0)
	/usr/local/Cellar/go/1.10.3/libexec/src/bufio/bufio.go:132 +0x3a
net/http.(*persistConn).readLoop(0xc4200cb320)
	/usr/local/Cellar/go/1.10.3/libexec/src/net/http/transport.go:1601 +0x185
created by net/http.(*Transport).dialConn
	/usr/local/Cellar/go/1.10.3/libexec/src/net/http/transport.go:1237 +0x95a

goroutine 31 [select]:
net/http.(*persistConn).writeLoop(0xc4200cb320)
	/usr/local/Cellar/go/1.10.3/libexec/src/net/http/transport.go:1822 +0x14b
created by net/http.(*Transport).dialConn
	/usr/local/Cellar/go/1.10.3/libexec/src/net/http/transport.go:1238 +0x97f

goroutine 48 [sleep]:
time.Sleep(0xb2d05e00)
	/usr/local/Cellar/go/1.10.3/libexec/src/runtime/time.go:102 +0x166
github.com/vmware/go-vcloud-director/govcd.(*Task).WaitTaskCompletion(0xc42014fb30, 0x0, 0x0)
	/Users/Uppundaa/go2/src/github.com/vmware/go-vcloud-director/govcd/task.go:75 +0x41
github.com/vmware/go-vcloud-director/govcd.(*TestVCD).Test_Shutdown(0xc4200206e0, 0xc42012a3c0)
	/Users/Uppundaa/go2/src/github.com/vmware/go-vcloud-director/govcd/vapp_test.go:236 +0xf4
reflect.Value.call(0x143f320, 0xc4200206e0, 0x9613, 0x1446cc2, 0x4, 0xc42004ef70, 0x1, 0x1, 0x16918e0, 0x143e120, ...)
	/usr/local/Cellar/go/1.10.3/libexec/src/reflect/value.go:447 +0x969
reflect.Value.Call(0x143f320, 0xc4200206e0, 0x9613, 0xc4204bff70, 0x1, 0x1, 0x0, 0xc4204bff07, 0x0)
	/usr/local/Cellar/go/1.10.3/libexec/src/reflect/value.go:308 +0xa4
github.com/vmware/go-vcloud-director/vendor/gopkg.in/check%2ev1.(*suiteRunner).forkTest.func1(0xc42012a3c0)
	/Users/Uppundaa/go2/src/github.com/vmware/go-vcloud-director/vendor/gopkg.in/check.v1/check.go:781 +0x575
github.com/vmware/go-vcloud-director/vendor/gopkg.in/check%2ev1.(*suiteRunner).forkCall.func1(0xc420126100, 0xc42012a3c0, 0xc42027ede0)
	/Users/Uppundaa/go2/src/github.com/vmware/go-vcloud-director/vendor/gopkg.in/check.v1/check.go:675 +0x7c
created by github.com/vmware/go-vcloud-director/vendor/gopkg.in/check%2ev1.(*suiteRunner).forkCall
	/Users/Uppundaa/go2/src/github.com/vmware/go-vcloud-director/vendor/gopkg.in/check.v1/check.go:672 +0x20c

rax    0x4
rbx    0x0
rcx    0x7fff5fbff050
rdx    0x0
rdi    0x4
rsi    0x0
rbp    0x7fff5fbff8c0
rsp    0x7fff5fbff050
r8     0x40
r9     0x0
r10    0x7fff5fbff0c0
r11    0x246
r12    0xc42014f188
r13    0x1
r14    0xc42008c9c0
r15    0x1
rip    0x105be73
rflags 0x247
cs     0x7
fs     0x0
gs     0x0
*** Test killed with quit: ran too long (10m0s).
exit status 2

Remove reliance on variable VCDVDCHREF

Since we want the sdk to be able to operate on any org or vdc that the user wants, we need to remove the VCDVDCHREF variable which stores the href of the vdc that the user specified during authentication. Instead we want the methods to use the href of the object the user is using to make calls to the rest api. With this we need to add getbyname methods for vdcs and orgs so that users can use whatever object they want. This will be the first step towards making the sdk usable for a broader scope instead of just within one orgvdc

Add logging and more error checking for all API calls

The API calls in the library are all route through this paradigm:

(1)
request := vcd.client.NewRequest(parameters)
This implementation skips explicitly the possible errors returned by http.NewRequest.
An error check (and logging) should be added to the function in api.go.

(2)
The APi is called using response, err := checkResp(v.c.Http.Do(request))
This call loses any visibility on the request, which can't be easily logged unless by adding it to every occurrence of checkResp
A possible improvement would be replacing checkResponse(response as result of http.Do) with RunRequest(client, request). This change would allow us to log the contents of the request for all calls, before processing the response

(3)
The response is processed inside checkResp and sometimes it is lost before we can see its raw contents. (See Issue #54)
We need to log the response within checkResp (or its proposed replacement RunRequest).

This refactoring would involve minimal global modification, with the logging handled in two or three places only.

Clean up all resources created during Tests in case of failure

We should add to our cleanup suite in api_vcd.go a check of all resources we have created during the test run. If anything is left remaining we should delete it. This allows users to rerun the tests if they fail without deleting the resource the tests created. Right now we only do that for the vapp that we create in the beginning of testing.

parseErr and decodeBody don't check for HTTP response body EOF

In api.go, the functions parseErr and decodeBody assume that the HTTP response body has always some content. Thus, they attempt to unmarshal the body without checking its length.
The result is that the error message contains incomplete information, such as

"error parsing error body for non-200 request: EOF"

// parseErr takes an error XML resp and returns a single string for use in error messages.
func parseErr(resp *http.Response) error {

    errBody := new(types.Error)

    // #############################################
    // ### before calling decodeBody, it should check for length
    // #############################################
    // if there was an error decoding the body, just return that
    if err := decodeBody(resp, errBody); err != nil {
        return fmt.Errorf("error parsing error body for non-200 request: %s", err)
    }

    return fmt.Errorf("API Error: %d: %s", errBody.MajorErrorCode, errBody.Message)
}

parseErr should check for empty body before calling decodeBody. Alternatively, it should handle the returned error and issue a more sensible error, for example giving the Response.Status, which at least suggest where to start investigating a failure.

Upload a Catalog Item

We need ability to upload OVAs to the catalog. The implementation is not trivial, because they way vCD API works. You first have to extract the OVA, because vCD first needs the OVF description which is inside. Then vCD returns a set of links (which represent the References->File entries in the OVF XML), to which you upload each file separately.

In addition to the basic OVA upload, the following is also needed:

  1. Support chunked OVA files.
  2. Wait for the vCD to finish the import after uploading (this is above what pyvcloud does Today).
  3. Utilise existing Task mechanism.
  4. Report percentage progress.

OVA usage on Testing.md

We need to add into Testing.md a disclaimer about specific OVAs. Some OVAs don't have guest_customization which means that our tests for guest_customization and set_ovf will fail

Independent disk Implementation

Independent disk operations are missing in go-vcloud-director:
Missing Operations:

  • Create Disk
  • Update Disk
  • Delete Disk
  • Attach Disk
  • Detach Disk

The Pull Request template is easy to miss

When you do a PR the commit message is copied over automatically into the PR message as can be seen below. The .github/PULL_REQUEST_TEMPLATE.md appears at the bottom and is easy to miss or even submit together with the message. We need to emphasise and put more attention to it.

pr template merges with the commit message

Add tests for new item in each main object

There are no tests related to New***() in most files.
Here's the list of missing ones:

  • NewCatalog()
  • NewCatalogItem()
  • NewEdgeGateway()
  • NewOrg()
  • NewOrgVDCNetwork()
  • NewResults()
  • NewTask()
  • NewVapp()
  • NewVAppTemplate()
  • NewVdc()
  • NewVM()

Given the similar implementation, I suspect tests for the above items could be created in a similar way.
Will investigate.

vapp test: check power status before cycle

In several tests, the vapp test runs an unconditional power on and off and then on again.
We should check the power status before running power ON. If such function is missing from the library, it should be implemented.

These tests should be updated:

  • Test_Reboot
  • Test_Reset
  • Test_Suspend
  • Test_Shutdown
  • Test_Poweroff

Add a logging module to the library.

Related to Issues #38 and #67, of which this one is a needed precondition, the library should adopt a consistent logging system.

Currently, there are sparse calls to Go standard log package, without any configuration. The default output for these calls is stderr, i.e. the terminal screen.

The log module should implement the following:

  • A global variable to enable logging, disabled by default.
  • A global variable for log output, so that we can define where to send the logs when we don't want them on the screen;
  • The log format (the default is 2009/11/10 23:00:00 message
  • (Optionally) The log level of detail (the default is INFO).

Add checks for final state to edge gateway test

The edge gateway test performs checks on the state of the error and the task, but it does not check the real state of the gateway itself.

To complete the test, we should add checks to verify the mappings in the gateway.

  • Test_NATMapping
  • Test_NATPortMapping
  • Test_1to1Mappings
  • Test_AddIpsecVPN (also test the operational status of the edge gateway after the operation)

Introduce name spaces for tests

Currently our test suite uses check.v1, which recommends squashing name spaces into the test code. This practice makes code reading harder, and should be avoided.

We should fix it immediately after we finish working on Issue #10.

Extend test to create the network

Right now we have an orgvdcnetwork_test.go, which tests only finding of the network. However, the library actually has functionality to create Org vDC networks right now. This can be tested by using Terraform vCD provider.

The test needs to be extended to actually create the network and then find the one that has been created.

After this change the following parameter in the tests' config.yaml should not require an existing network, rather it should depict how to name the network that will be created by tests or, if possible, we can drop it all-together and make configuration simpler:
network: TerraformNetwork

Add check for empty Org/Vdc in SetupTestSuite

Since the Get functions changed format (issue #69), we need to add an extra check to check if the org/vdc is empty. If empty we need to panic to prevent a segmentation fault.

	// set org
	vcd.org, err = GetOrgByName(vcd.client, config.VCD.Org)
	if err != nil {
		panic(err)
	}
	// set vdc
	vcd.vdc, err = vcd.org.GetVdcByName(config.VCD.Vdc)
	if err != nil {
		panic(err)
	}

Change these to check for vcd.org == (Org{}) and vcd.vdc == (Vdc{}). If this is the case that means that no org/vdc was found, so we have to panic.

Enable Travis builds for project

Add Travis build to run go build as well as check formatting of code. The following should generate build failures and block PRs:

For this to work the project must be a public repo. Here's one of a number of articles on Travis CI/Github integration: https://hackernoon.com/continuous-integration-using-travis-on-github-1f7f2314b6b7

Ability for AdminOrg objects to call org functions

We need to discuss whether we want to fill up the org variable as well in the admin org. It will take an extra rest api call to get but it will enable users to be able to make calls to org functions as well. This is probably the closest we can get to pure inheritance.

vapp_test: investigate constraint on Test_Undeploy

In Test_Undeploy, a comment warns that a re-deployment is needed for the test to succeed, and recommends to remove the redundant operation eventually.

The reason for the failure should be investigated and the test amended to work without unnecessary operations.

vapp_test: check effects on object in addition to errors

In vapp_test, several tests run only a check on the status of the error or the task, without checking the effects on the object being tested:

  • Test_SetOvf: check that the changes on the OVF have been applied;
  • Test_AddMetadata: check that the metadata was changed, and add checks with valid and invalid metadata.
  • Test_RunCustomizationScript: check that the script is valid, and its effects are applied to the vApp.
  • Test_ChangeCpuCount: check that the CPU count did change after the operation.
  • Test_ChangeMemorySize: check that the memory size has changed by comparing memory amount before and after.
  • Test_ChangeStorageProfile: make sure that the vApp uses the new storage profile.
  • Test_ChangeVMName: check that the name of the VM was changed.

Improve test_vdc with checks on appropriate values

The checks in test_vdc were made (using a mock server) by testing that the mock objects had the expected values, which were mostly made up.
The real test should:

  • Check the acceptable values for AllocationModel and make sure that the vDC has one of them;
  • Check what constitute a valid ComputeCapacity and make sure the VMs values are within the expected ranges.

Revise error handling to make it easier for clients to distinguish function outcomes

Error handling in go-vcloud-director does not clearly distinguish clearly between different types of errors:

  • Not finding something
  • Failing due to a client-side error like bad host name
  • Failing due to a problem on the server, e.g., not authorized, bad input, or a server failure

These are distinct outputs that require different handling; the first output may not even be an error. In our current model clients must check strings to determine exactly what happened. Here’s an example: finding Orgs by name. In the current implementation two things can happen:

  • You find an Org and return it.
  • Anything else is an error whose exact nature is denoted by the string value of the error message.
    The following code from system.go illustrates:
func GetOrgByName(c *VCDClient, orgname string) (Org, error) {
        OrgHREF, err := getOrgHREF(c, orgname)
        if err != nil {
                return Org{}, fmt.Errorf("Cannot find the url of the org: %s", err)
        }
        u, err := url.ParseRequestURI(OrgHREF)
        if err != nil {
                return Org{}, fmt.Errorf("Error parsing org href: %v", err)
        }
        req := c.Client.NewRequest(map[string]string{}, "GET", *u, nil)
        resp, err := checkResp(c.Client.Http.Do(req))
        if err != nil {
                return Org{}, fmt.Errorf("error retreiving org: %s", err)
        }

        o := NewOrg(&c.Client)
        if err = decodeBody(resp, o.Org); err != nil {
                return Org{}, fmt.Errorf("error decoding org response: %s", err)
        }
        return *o, nil
}

It's hard to write a client that uses this call to see if an Org already exists, because you can't easily tell the difference between the call failing and the fact that the Org is not there. This is because you have to parse a string that could change and that you might not have right in the first place.

We need to look carefully at our function outputs. In general a function like the preceding example should do one of the following:

  • If you find the org return it.
  • If the org is not there return nil.
  • If you return an error return a typed value that corresponds (if possible) to the HTTP status. HTTP status codes are well-understood and easily extractable from any error. We can add a few client side codes as well. But the point is that the error should be a type that the compiler can confirm so that you have proof your client is looking for something that is actually there.

In short, we should think of errors as a specific interface that has a form like the following:

type VcdError struct {
    status int  // HTTP status code (could also be enum)
    reason string // HTTP reason (e.g., "Bad Request")
    msg string // full description of error
}

func (e *VcdError) Error() string { return e.msg }

See https://blog.golang.org/error-handling-and-go for more. These conventions will lead to client code that is more flexible and more likely to be correct.

Add an "how to write a new test" guide

Add a new guide explaining how to write a test that integrates into our suite.

The guide could be either stand-alone or a new section inside TESTING.md

Main topics:

  • Which imports are needed;
  • How to write a test function integrated into the existing suite;
  • What is the structure of TestVCD and what contents to expect;
  • What is check.v1 and what it can do for us;
  • Guidelines on testing code readability.

This issue depends on Issue#34, and should not start before its completion.

Clean-up code of one-letter identifiers

The whole code base is littered with identifiers (variables, struct types, and types) made of a single letter.

To make the library code more readable, and easier to maintain and expand, these identifiers must be replaced with more telling names.

The only place where single letter variables can be accepted is for a counter in a short loop.

Moreover, variables and field name vdc and vcd should be avoided as they are easily mixed up, and replaced with vcDirector and vdCenter or similarly telling names.

Add log setting to dump failing HTTP request/response values using a ring buffer

Diagnosing HTTP call failures often requires detailed logging of the request/response. The problem is that such logging is either feast or famine:

  • HTTP logging is turned off so there's no additional information beyond the error type itself
  • HTTP logging is turned on so the log is flooded with information that is useless if there are no errors. Even when there's an error you need to trek through irrelevant information to get to the error.

As a result users tend to keep logging off and then have to repeat operations with logging on once an error appears. This is inefficient and does not help with errors that appear intermittently.

We can address this problem using an in-memory ring buffer to hold the last N request/responses. If an error occurs we dump the ring buffer as if logging had been turned on. This gives clients diagnostic information without requiring HTTP logging to be turned on.

There are some subtleties in the implementation that must be considered.

  • If HTTP logging is turned on the ring buffer is disabled.
  • The ring buffer keeps entries in memory, so they cannot be large. Binary data like OVA uploads should not be stored.
  • Clients should be able to control the number of entries to keep and dump in the event of an error. Just keeping the last request is a good default.
  • Each Client struct should have its own ring buffer to prevent confusion in concurrent applications.

Golang has a ring implementation that may be helpful for implementation. See https://golang.org/pkg/container/ring/ for more information.

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.