Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[specific ci=Group23-Vic-Machine-Service] syslog support added to VCH create and inspect API #6582

Merged
merged 7 commits into from
Oct 24, 2017
Merged

[specific ci=Group23-Vic-Machine-Service] syslog support added to VCH create and inspect API #6582

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
a3b3132
syslog support added to VCH create API
AngieCris Oct 17, 2017
e518893
syslog addr added to inspect output
AngieCris Oct 18, 2017
dd03e5b
syslog_addr format changed in swagger
AngieCris Oct 19, 2017
17e3514
use status code in http package
AngieCris Oct 19, 2017
77eee58
empty syslog_addr string checking
AngieCris Oct 19, 2017
16c412c
swagger format changed for syslog_addr
AngieCris Oct 19, 2017
56fb57a
roll back in swagger: only tcp and udp are accepted
AngieCris Oct 20, 2017
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
12 changes: 6 additions & 6 deletions cmd/vic-machine/create/create.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ type Create struct {

Proxies common.Proxies

syslogAddr string
SyslogAddr string

executor *management.Dispatcher
}
Expand Down Expand Up @@ -271,7 +271,7 @@ func (c *Create) Flags() []cli.Flag {
Name: "syslog-address",
Value: "",
Usage: "Address of the syslog server to send Virtual Container Host logs to. Must be in the format transport://host[:port], where transport is udp or tcp. port defaults to 514 if not specified",
Destination: &c.syslogAddr,
Destination: &c.SyslogAddr,
Hidden: true,
},
}
Expand Down Expand Up @@ -416,7 +416,7 @@ func (c *Create) processParams() error {
c.HTTPProxy = hproxy
c.HTTPSProxy = sproxy

if err := c.processSyslog(); err != nil {
if err := c.ProcessSyslog(); err != nil {
return err
}

Expand Down Expand Up @@ -543,12 +543,12 @@ func (c *Create) ProcessNetwork(network *data.NetworkConfig, netName, pgName, st
return nil
}

func (c *Create) processSyslog() error {
if len(c.syslogAddr) == 0 {
func (c *Create) ProcessSyslog() error {
if len(c.SyslogAddr) == 0 {
return nil
}

u, err := url.Parse(c.syslogAddr)
u, err := url.Parse(c.SyslogAddr)
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion lib/apiservers/service/restapi/handlers/vch_cert_get.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func getVCHCert(op trace.Operation, d *data.Data) (*config.RawCertificate, error
}

if vchConfig.HostCertificate.IsNil() {
return nil, util.NewError(404, fmt.Sprintf("No certificate found for VCH %s", d.ID))
return nil, util.NewError(http.StatusNotFound, fmt.Sprintf("No certificate found for VCH %s", d.ID))
}

return vchConfig.HostCertificate, nil
Expand Down
64 changes: 36 additions & 28 deletions lib/apiservers/service/restapi/handlers/vch_create.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"io"
"math"
"net"
"net/http"
"net/url"
"os"
"path"
Expand Down Expand Up @@ -79,7 +80,7 @@ func (h *VCHCreate) Handle(params operations.PostTargetTargetVchParams, principa

validator, err := validateTarget(params.HTTPRequest.Context(), d)
if err != nil {
return operations.NewPostTargetTargetVchDefault(400).WithPayload(&models.Error{Message: err.Error()})
return operations.NewPostTargetTargetVchDefault(http.StatusBadRequest).WithPayload(&models.Error{Message: err.Error()})
Copy link
Member

Choose a reason for hiding this comment

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

I think this is a really good change. Can you file an issue so that we remember to clean this up in the rest of the handlers?

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 can do it in this pr

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixes done

}

c, err := buildCreate(params.HTTPRequest.Context(), d, validator.Session.Finder, params.Vch)
Expand Down Expand Up @@ -115,7 +116,7 @@ func (h *VCHDatacenterCreate) Handle(params operations.PostTargetTargetDatacente

validator, err := validateTarget(params.HTTPRequest.Context(), d)
if err != nil {
return operations.NewPostTargetTargetDatacenterDatacenterVchDefault(400).WithPayload(&models.Error{Message: err.Error()})
return operations.NewPostTargetTargetDatacenterDatacenterVchDefault(http.StatusBadRequest).WithPayload(&models.Error{Message: err.Error()})
}

c, err := buildCreate(params.HTTPRequest.Context(), d, validator.Session.Finder, params.Vch)
Expand Down Expand Up @@ -164,17 +165,17 @@ func buildCreate(ctx context.Context, d *data.Data, finder *find.Finder, vch *mo

if vch != nil {
if vch.Version != "" && version.String() != string(vch.Version) {
return nil, util.NewError(400, fmt.Sprintf("Invalid version: %s", vch.Version))
return nil, util.NewError(http.StatusBadRequest, fmt.Sprintf("Invalid version: %s", vch.Version))
}

c.DisplayName = vch.Name

// TODO: move validation to swagger
if err := common.CheckUnsupportedChars(c.DisplayName); err != nil {
return nil, util.NewError(400, fmt.Sprintf("Invalid display name: %s", err))
return nil, util.NewError(http.StatusBadRequest, fmt.Sprintf("Invalid display name: %s", err))
}
if len(c.DisplayName) > create.MaxDisplayNameLen {
return nil, util.NewError(400, fmt.Sprintf("Invalid display name: length exceeds %d characters", create.MaxDisplayNameLen))
return nil, util.NewError(http.StatusBadRequest, fmt.Sprintf("Invalid display name: length exceeds %d characters", create.MaxDisplayNameLen))
}

debug := int(vch.Debug)
Expand All @@ -195,10 +196,10 @@ func buildCreate(ctx context.Context, d *data.Data, finder *find.Finder, vch *mo

resourcePath, err := fromManagedObject(ctx, finder, "ResourcePool", vch.Compute.Resource) // TODO: Do we need to handle clusters differently?
if err != nil {
return nil, util.NewError(400, fmt.Sprintf("Error finding resource pool: %s", err))
return nil, util.NewError(http.StatusBadRequest, fmt.Sprintf("Error finding resource pool: %s", err))
}
if resourcePath == "" {
return nil, util.NewError(400, "Resource pool must be specified (by name or id)")
return nil, util.NewError(http.StatusBadRequest, "Resource pool must be specified (by name or id)")
}
c.ComputeResourcePath = resourcePath
}
Expand All @@ -207,67 +208,67 @@ func buildCreate(ctx context.Context, d *data.Data, finder *find.Finder, vch *mo
if vch.Network.Bridge != nil {
path, err := fromManagedObject(ctx, finder, "Network", vch.Network.Bridge.PortGroup)
if err != nil {
return nil, util.NewError(400, fmt.Sprintf("Error finding bridge network: %s", err))
return nil, util.NewError(http.StatusBadRequest, fmt.Sprintf("Error finding bridge network: %s", err))
}
if path == "" {
return nil, util.NewError(400, "Bridge network portgroup must be specified (by name or id)")
return nil, util.NewError(http.StatusBadRequest, "Bridge network portgroup must be specified (by name or id)")
}
c.BridgeNetworkName = path
c.BridgeIPRange = fromCIDR(&vch.Network.Bridge.IPRange.CIDR)

if err := c.ProcessBridgeNetwork(); err != nil {
return nil, util.WrapError(400, err)
return nil, util.WrapError(http.StatusBadRequest, err)
}
}

if vch.Network.Client != nil {
path, err := fromManagedObject(ctx, finder, "Network", vch.Network.Client.PortGroup)
if err != nil {
return nil, util.NewError(400, fmt.Sprintf("Error finding client network portgroup: %s", err))
return nil, util.NewError(http.StatusBadRequest, fmt.Sprintf("Error finding client network portgroup: %s", err))
}
if path == "" {
return nil, util.NewError(400, "Client network portgroup must be specified (by name or id)")
return nil, util.NewError(http.StatusBadRequest, "Client network portgroup must be specified (by name or id)")
}
c.ClientNetworkName = path
c.ClientNetworkGateway = fromGateway(vch.Network.Client.Gateway)
c.ClientNetworkIP = fromNetworkAddress(vch.Network.Client.Static)

if err := c.ProcessNetwork(&c.Data.ClientNetwork, "client", c.ClientNetworkName, c.ClientNetworkIP, c.ClientNetworkGateway); err != nil {
return nil, util.WrapError(400, err)
return nil, util.WrapError(http.StatusBadRequest, err)
}
}

if vch.Network.Management != nil {
path, err := fromManagedObject(ctx, finder, "Network", vch.Network.Management.PortGroup)
if err != nil {
return nil, util.NewError(400, fmt.Sprintf("Error finding management network portgroup: %s", err))
return nil, util.NewError(http.StatusBadRequest, fmt.Sprintf("Error finding management network portgroup: %s", err))
}
if path == "" {
return nil, util.NewError(400, "Management network portgroup must be specified (by name or id)")
return nil, util.NewError(http.StatusBadRequest, "Management network portgroup must be specified (by name or id)")
}
c.ManagementNetworkName = path
c.ManagementNetworkGateway = fromGateway(vch.Network.Management.Gateway)
c.ManagementNetworkIP = fromNetworkAddress(vch.Network.Management.Static)

if err := c.ProcessNetwork(&c.Data.ManagementNetwork, "management", c.ManagementNetworkName, c.ManagementNetworkIP, c.ManagementNetworkGateway); err != nil {
return nil, util.WrapError(400, err)
return nil, util.WrapError(http.StatusBadRequest, err)
}
}

if vch.Network.Public != nil {
path, err := fromManagedObject(ctx, finder, "Network", vch.Network.Public.PortGroup)
if err != nil {
return nil, util.NewError(400, fmt.Sprintf("Error finding public network portgroup: %s", err))
return nil, util.NewError(http.StatusBadRequest, fmt.Sprintf("Error finding public network portgroup: %s", err))
}
if path == "" {
return nil, util.NewError(400, "Public network portgroup must be specified (by name or id)")
return nil, util.NewError(http.StatusBadRequest, "Public network portgroup must be specified (by name or id)")
}
c.PublicNetworkName = path
c.PublicNetworkGateway = fromGateway(vch.Network.Public.Gateway)
c.PublicNetworkIP = fromNetworkAddress(vch.Network.Public.Static)

if err := c.ProcessNetwork(&c.Data.PublicNetwork, "public", c.PublicNetworkName, c.PublicNetworkIP, c.PublicNetworkGateway); err != nil {
return nil, util.WrapError(400, err)
return nil, util.WrapError(http.StatusBadRequest, err)
}
}

Expand All @@ -284,17 +285,17 @@ func buildCreate(ctx context.Context, d *data.Data, finder *find.Finder, vch *mo

path, err := fromManagedObject(ctx, finder, "Network", cnetwork.PortGroup)
if err != nil {
return nil, util.NewError(400, fmt.Sprintf("Error finding portgroup for container network %s: %s", alias, err))
return nil, util.NewError(http.StatusBadRequest, fmt.Sprintf("Error finding portgroup for container network %s: %s", alias, err))
}
if path == "" {
return nil, util.NewError(400, fmt.Sprintf("Container network %s portgroup must be specified (by name or id)", alias))
return nil, util.NewError(http.StatusBadRequest, fmt.Sprintf("Container network %s portgroup must be specified (by name or id)", alias))
}
containerNetworks.MappedNetworks[alias] = path

address := net.ParseIP(string(cnetwork.Gateway.Address))
_, mask, err := net.ParseCIDR(string(cnetwork.Gateway.RoutingDestinations[0].CIDR))
if err != nil {
return nil, util.NewError(400, fmt.Sprintf("Error parsing network mask for container network %s: %s", alias, err))
return nil, util.NewError(http.StatusBadRequest, fmt.Sprintf("Error parsing network mask for container network %s: %s", alias, err))
}
containerNetworks.MappedNetworksGateways[alias] = net.IPNet{
IP: address,
Expand Down Expand Up @@ -327,7 +328,7 @@ func buildCreate(ctx context.Context, d *data.Data, finder *find.Finder, vch *mo
}

if err := common.CheckUnsupportedCharsDatastore(c.ImageDatastorePath); err != nil {
return nil, util.WrapError(400, err)
return nil, util.WrapError(http.StatusBadRequest, err)
}

if vch.Storage.VolumeStores != nil {
Expand All @@ -339,7 +340,7 @@ func buildCreate(ctx context.Context, d *data.Data, finder *find.Finder, vch *mo
vs := common.VolumeStores{VolumeStores: cli.StringSlice(volumes)}
volumeLocations, err := vs.ProcessVolumeStores()
if err != nil {
return nil, util.NewError(400, fmt.Sprintf("Error processing volume stores: %s", err))
return nil, util.NewError(http.StatusBadRequest, fmt.Sprintf("Error processing volume stores: %s", err))
}
c.VolumeLocations = volumeLocations
}
Expand Down Expand Up @@ -367,7 +368,7 @@ func buildCreate(ctx context.Context, d *data.Data, finder *find.Finder, vch *mo
c.Certs.KeySize = fromValueBits(vch.Auth.Server.Generate.Size)

if err := c.Certs.ProcessCertificates(c.DisplayName, c.Force, 0); err != nil {
return nil, util.NewError(400, fmt.Sprintf("Error generating certificates: %s", err))
return nil, util.NewError(http.StatusBadRequest, fmt.Sprintf("Error generating certificates: %s", err))
}
} else {
c.Certs.CertPEM = []byte(vch.Auth.Server.Certificate.Pem)
Expand Down Expand Up @@ -397,7 +398,7 @@ func buildCreate(ctx context.Context, d *data.Data, finder *find.Finder, vch *mo
}
}
if err := c.OpsCredentials.ProcessOpsCredentials(true, c.Target.User, c.Target.Password); err != nil {
return nil, util.WrapError(400, err)
return nil, util.WrapError(http.StatusBadRequest, err)
}
}

Expand All @@ -413,10 +414,17 @@ func buildCreate(ctx context.Context, d *data.Data, finder *find.Finder, vch *mo
c.Proxies = fromImageFetchProxy(vch.Registry.ImageFetchProxy)
_, _, err := c.Proxies.ProcessProxies()
if err != nil {
return nil, util.NewError(400, fmt.Sprintf("Error processing proxies: %s", err))
return nil, util.NewError(http.StatusBadRequest, fmt.Sprintf("Error processing proxies: %s", err))
}
}
}

if vch.SyslogAddr != "" {
c.SyslogAddr = vch.SyslogAddr.String()
if err := c.ProcessSyslog(); err != nil {
return nil, util.NewError(http.StatusBadRequest, fmt.Sprintf("Error processing syslog server address: %s", err))
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why can't you write a simple unit test to verify these new code paths?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is part of the create API handler and we haven't figured out a pattern to do unit tests on swagger handlers yet.
The new code path validates a field in http GET request JSON, and it's very hard to test it separately from all the other fields related to vch create params.
We're planning to integrate this in unit tests for create API as a whole (the tracking story is: #6018)

}

return c, nil
Expand All @@ -435,7 +443,7 @@ func handleCreate(ctx context.Context, c *create.Create, validator *validate.Val
executor := management.NewDispatcher(validator.Context, validator.Session, nil, false)
err = executor.CreateVCH(vchConfig, vConfig)
if err != nil {
return nil, util.NewError(500, fmt.Sprintf("Failed to create VCH: %s", err))
return nil, util.NewError(http.StatusInternalServerError, fmt.Sprintf("Failed to create VCH: %s", err))
}

return nil, nil
Expand Down
14 changes: 10 additions & 4 deletions lib/apiservers/service/restapi/handlers/vch_get.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"encoding/pem"
"fmt"
"net"
"net/http"
"net/url"
"strings"

Expand Down Expand Up @@ -98,23 +99,23 @@ func (h *VCHDatacenterGet) Handle(params operations.GetTargetTargetDatacenterDat
func getVCH(ctx context.Context, d *data.Data) (*models.VCH, error) {
validator, err := validateTarget(ctx, d)
if err != nil {
return nil, util.WrapError(400, err)
return nil, util.WrapError(http.StatusBadRequest, err)
}

executor := management.NewDispatcher(validator.Context, validator.Session, nil, false)
vch, err := executor.NewVCHFromID(d.ID)
if err != nil {
return nil, util.NewError(500, fmt.Sprintf("Failed to inspect VCH: %s", err))
return nil, util.NewError(http.StatusInternalServerError, fmt.Sprintf("Failed to inspect VCH: %s", err))
}

err = validate.SetDataFromVM(validator.Context, validator.Session.Finder, vch, d)
if err != nil {
return nil, util.NewError(500, fmt.Sprintf("Failed to load VCH data: %s", err))
return nil, util.NewError(http.StatusInternalServerError, fmt.Sprintf("Failed to load VCH data: %s", err))
}

model, err := vchToModel(vch, d, executor)
if err != nil {
return nil, util.WrapError(500, err)
return nil, util.WrapError(http.StatusInternalServerError, err)
}

return model, nil
Expand Down Expand Up @@ -260,6 +261,11 @@ func vchToModel(vch *vm.VirtualMachine, d *data.Data, executor *management.Dispa
}
}

// syslog_addr: syslog server address
if syslogConfig := vchConfig.Diagnostics.SysLogConfig; syslogConfig != nil {
model.SyslogAddr = strfmt.URI(syslogConfig.Network + "://" + syslogConfig.RAddr)
}

return model, nil
}

Expand Down
5 changes: 3 additions & 2 deletions lib/apiservers/service/restapi/handlers/vch_list_get.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package handlers
import (
"context"
"fmt"
"net/http"
"net/url"
"path"

Expand Down Expand Up @@ -83,13 +84,13 @@ func (h *VCHDatacenterListGet) Handle(params operations.GetTargetTargetDatacente
func listVCHs(ctx context.Context, d *data.Data) ([]*models.VCHListItem, error) {
validator, err := validateTarget(ctx, d)
if err != nil {
return nil, util.WrapError(400, err)
return nil, util.WrapError(http.StatusBadRequest, err)
}

executor := management.NewDispatcher(validator.Context, validator.Session, nil, false)
vchs, err := executor.SearchVCHs(validator.ClusterPath)
if err != nil {
return nil, util.NewError(500, fmt.Sprintf("Failed to search VCHs in %s: %s", validator.ResourcePoolPath, err))
return nil, util.NewError(http.StatusInternalServerError, fmt.Sprintf("Failed to search VCHs in %s: %s", validator.ResourcePoolPath, err))
}

return vchsToModels(ctx, vchs, executor), nil
Expand Down
5 changes: 5 additions & 0 deletions lib/apiservers/service/swagger.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -798,6 +798,11 @@
"type": "string"
}
}
},
"syslog_addr": {
"type": "string",
"format": "uri",
"pattern": "^(tc|ud)p:\/\/.*"
}
}
},
Expand Down