Skip to content

Commit

Permalink
APP-6182 validate model triplets in meta.json (#4369)
Browse files Browse the repository at this point in the history
  • Loading branch information
abe-winter committed Sep 17, 2024
1 parent 9281ff5 commit 909d47b
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 7 deletions.
42 changes: 41 additions & 1 deletion cli/module_registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,15 @@ import (
"go.viam.com/rdk/module"
"go.viam.com/rdk/module/modmanager"
modmanageroptions "go.viam.com/rdk/module/modmanager/options"
"go.viam.com/rdk/resource"
"go.viam.com/rdk/utils"
)

// moduleUploadChunkSize sets the number of bytes included in each chunk of the upload stream.
var moduleUploadChunkSize = 32 * 1024
var (
moduleUploadChunkSize = 32 * 1024
rdkAPITypes = []string{resource.APITypeServiceName, resource.APITypeComponentName}
)

// moduleVisibility determines whether modules are public or private.
type moduleVisibility string
Expand All @@ -46,6 +50,14 @@ const (
moduleVisibilityPublic moduleVisibility = "public"
)

type unknownRdkAPITypeError struct {
APIType string
}

func (err unknownRdkAPITypeError) Error() string {
return fmt.Sprintf("API with unknown type '%s', expected one of %s", err.APIType, strings.Join(rdkAPITypes, ", "))
}

// ModuleComponent represents an api - model pair.
type ModuleComponent struct {
API string `json:"api"`
Expand Down Expand Up @@ -199,6 +211,8 @@ func UpdateModuleAction(c *cli.Context) error {
return err
}

validateModels(c.App.ErrWriter, &manifest)

response, err := client.updateModule(moduleID, manifest)
if err != nil {
return err
Expand Down Expand Up @@ -283,6 +297,8 @@ func UploadModuleAction(c *cli.Context) error {
return err
}

validateModels(c.App.ErrWriter, &manifest)

_, err = client.updateModule(moduleID, manifest)
if err != nil {
return errors.Wrap(err, "Module update failed. Please correct the following issues in your meta.json")
Expand Down Expand Up @@ -316,6 +332,30 @@ func UploadModuleAction(c *cli.Context) error {
return nil
}

// call validateModelAPI on all models in manifest and warn if violations.
func validateModels(errWriter io.Writer, manifest *moduleManifest) {
for _, model := range manifest.Models {
if err := validateModelAPI(model.API); err != nil {
warningf(errWriter, "error validating API string %s: %s", model.API, err)
}
}
}

// return a useful error if the model string looks wrong.
func validateModelAPI(modelAPI string) error {
api, err := resource.ParseAPIString(modelAPI)
if err != nil {
return errors.Wrap(err, "unparseable model string")
}
if err := api.Validate(); err != nil {
return errors.Wrap(err, "failed to validate API")
}
if !slices.Contains(rdkAPITypes, api.Type.Name) {
return unknownRdkAPITypeError{APIType: api.Type.Name}
}
return nil
}

// UpdateModelsAction figures out the models that a module supports and updates it's metadata file.
func UpdateModelsAction(c *cli.Context) error {
logger := logging.NewLogger("x")
Expand Down
17 changes: 17 additions & 0 deletions cli/module_registry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,3 +47,20 @@ func TestUpdateModelsAction(t *testing.T) {

test.That(t, sameModels(metaModels.Models, expectedMetaModels.Models), test.ShouldBeTrue)
}

func TestValidateModelAPI(t *testing.T) {
err := validateModelAPI("rdk:component:x")
test.That(t, err, test.ShouldBeNil)
err = validateModelAPI("rdk:service:x")
test.That(t, err, test.ShouldBeNil)
err = validateModelAPI("rdk:unknown:x")
test.That(t, err, test.ShouldHaveSameTypeAs, unknownRdkAPITypeError{})
err = validateModelAPI("other:unknown:x")
test.That(t, err, test.ShouldHaveSameTypeAs, unknownRdkAPITypeError{})
err = validateModelAPI("rdk:component")
test.That(t, err, test.ShouldNotBeNil)
err = validateModelAPI("other:component:$x")
test.That(t, err, test.ShouldNotBeNil)
err = validateModelAPI("other:component:x_")
test.That(t, err, test.ShouldBeNil)
}
21 changes: 15 additions & 6 deletions resource/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,18 +125,27 @@ func (a API) MarshalJSON() ([]byte, error) {
return json.Marshal(a.String())
}

// ParseAPIString builds an API{} struct from a colon-delimited triple.
func ParseAPIString(apiStr string) (API, error) {
ret := API{}
matches := apiRegexValidator.FindStringSubmatch(apiStr)
if matches == nil {
return ret, fmt.Errorf("not a valid API config string. Input: `%v`", apiStr)
}
return APINamespace(matches[1]).WithType(matches[2]).WithSubtype(matches[3]), nil
}

// UnmarshalJSON parses either a string of the form namespace:type:subtype or a json object into an
// API object.
func (a *API) UnmarshalJSON(data []byte) error {
var apiStr string
if err := json.Unmarshal(data, &apiStr); err == nil {
// If the value is a string, regex match for a colon partitioned triplet.
if !apiRegexValidator.MatchString(apiStr) {
return fmt.Errorf("not a valid API config string. Input: `%v`", string(data))
// If the value is a string, parse it.
parsed, err := ParseAPIString(apiStr)
if err != nil {
return err
}

matches := apiRegexValidator.FindStringSubmatch(apiStr)
*a = APINamespace(matches[1]).WithType(matches[2]).WithSubtype(matches[3])
*a = parsed
return nil
}

Expand Down

0 comments on commit 909d47b

Please sign in to comment.