Skip to content
This repository has been archived by the owner on Nov 8, 2022. It is now read-only.

Snap doesn't return error on invalid task file #1157

Closed
thomastaylor312 opened this issue Aug 23, 2016 · 9 comments
Closed

Snap doesn't return error on invalid task file #1157

thomastaylor312 opened this issue Aug 23, 2016 · 9 comments

Comments

@thomastaylor312
Copy link
Contributor

When creating a task from a JSON file, Snap ignores invalid configuration

Bad config file:

{
   "version":1,
   "schedule":{
      "type":"simple",
      "interval":"1s"
   },
   "workflow":{
      "collect":{
         "metrics":{
            "/intel/procfs/disk/*/*":{

            }
         }
      },
      "process":null,
      "publish":{
         "plugin_name":"file",
         "config":{
            "file":"/tmp/testoutput"
         }
      }
   }
}

When you create the task, it creates successfully with the following output from snapctl task export:

{
   "id":"3c972277-60e1-4339-8a43-74539bdc3d94",
   "name":"Task-3c972277-60e1-4339-8a43-74539bdc3d94",
   "deadline":"5s",
   "workflow":{
      "collect":{
         "metrics":{
            "/intel/procfs/disk/*/*":{
               "version":0
            }
         }
      }
   },
   "schedule":{
      "type":"simple",
      "interval":"1s"
   },
   "creation_timestamp":1471977934,
   "last_run_timestamp":1471977973,
   "hit_count":39,
   "task_state":"Running",
   "href":"http://localhost:8181/v1/tasks/3c972277-60e1-4339-8a43-74539bdc3d94",
   "Err":null
}

It appears that snap ignored the invalid parts of the workflow instead of erroring on creation. This is tested against the latest master

@IRCody
Copy link
Contributor

IRCody commented Aug 24, 2016

@thomastaylor312: You are talking about the Publish not being a list correct? (It took me a few to notice). I wonder if that's json marshaling not returning an error or if we are ignoring it?

@thomastaylor312
Copy link
Contributor Author

Yes, and it also happened when my publish was outside of the workflow object/dictionary

@thomastaylor312
Copy link
Contributor Author

Sorry, I meant that the publish was outside of the collect object/dictionary

@thomastaylor312
Copy link
Contributor Author

@IRCody The invalid task manifest is valid JSON, it is just an invalid task manifest

@IRCody
Copy link
Contributor

IRCody commented Aug 24, 2016

@thomastaylor312: Even if it's validJSON marshaling it into the task object should fail I think. I will try to look into it.

@geauxvirtual
Copy link
Contributor

Unless specific code is written to validate the task manifest sections, json.Unmarshal will just ignore anything that doesn't exist in the object that was passed to it.

@IRCody
Copy link
Contributor

IRCody commented Aug 24, 2016

@geauxvirtual: Thanks! That would explain the behavior then I think then. It matches what we are seeing where the task is created with only the portion of the task manifest that maps correctly to something in the task struct. I'm not sure the best way to fix it though, even with custom unmarshal there are limits to what you can check for I think.

@geauxvirtual
Copy link
Contributor

geauxvirtual commented Aug 25, 2016 via email

@geauxvirtual
Copy link
Contributor

In the current code I'm working on, I have this being outputted as an error message with the invalid task manifest supplied up above.

--- intelsdi-x/snap ‹tb/1130* ?› » go run cmd/snapctl/*.go task create -t test.json
Using task manifest to create task
Error parsing JSON file input - Unrecognized key 'process' in workflow of task.

exit status 1

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants