write out metadata json and plist files to root install directory #1417
Conversation
cmd/launcher/launcher.go
Outdated
| @@ -193,6 +193,9 @@ func runLauncher(ctx context.Context, cancel func(), opts *launcher.Options) err | |||
| // we expect we're live. Record the version for osquery to | |||
| // pickup | |||
| internal.RecordLauncherVersion(rootDirectory) | |||
| if err = internal.RecordMetadata(rootDirectory, ctx, k); err != nil { | |||
There was a problem hiding this comment.
Are we confident we'll have all this data on a fresh install since it has to come down via control server? Maybe that's okay and it will get written on the next launcher start up? or perhaps we would to do some retry logic in a goroutine.
There was a problem hiding this comment.
discussed offline^ my testing may have been flawed here- adding some async retry, thank you!
There was a problem hiding this comment.
Hrm... if we're really concerned about that, I wonder if we should move the write into the server data consumer.
There was a problem hiding this comment.
yea, this ☝️ is better
There was a problem hiding this comment.
actually, a subscriber makes more sense
Co-authored-by: James Pickett <James-Pickett@users.noreply.github.com>
|
|
||
| // RecordMetadata writes out both a json and plist (for darwin) file including all information | ||
| // in the metadata struct to the root install directory | ||
| func RecordMetadata(rootDir string, ctx context.Context, knapsack types.Knapsack, logger log.Logger) { |
There was a problem hiding this comment.
ctx should be first, logger second.
is rootDir in knapsack?
There was a problem hiding this comment.
cleaning this up, yes it should be just making sure it is initialized by then
| // RecordMetadata writes out both a json and plist (for darwin) file including all information | ||
| // in the metadata struct to the root install directory | ||
| func RecordMetadata(rootDir string, ctx context.Context, knapsack types.Knapsack, logger log.Logger) { | ||
| logger = log.With(logger, "method") |
There was a problem hiding this comment.
🤦 will fix this
| } | ||
|
|
||
| if err := backoff.WaitFor(func() error { | ||
| sdc := checkups.NewServerDataCheckup(knapsack) |
There was a problem hiding this comment.
I'd probably fish this out of sdc.k.ServerProvidedDataStore() instead of going through checkup
| Version: version.Version().Version, | ||
| } | ||
|
|
||
| if err := backoff.WaitFor(func() error { |
There was a problem hiding this comment.
What is backoff bringing?
Co-authored-by: seph <seph@kolide.co>
Co-authored-by: seph <seph@kolide.co>
|
|
||
| type metadata struct { | ||
| DeviceId string `json:"device_id" plist:"device_id"` | ||
| OrganizationId string `json:"organization_id" plist:"organization_id"` |
There was a problem hiding this comment.
Can you add OrganizationMunemo too?
| if err != nil { | ||
| return err | ||
| } else if string(deviceId) == "" { | ||
| return errors.New("device_id is not yet present in ServerProvidedDataStore") | ||
| } |
There was a problem hiding this comment.
dont need the else if here since you're returning
| if err != nil { | |
| return err | |
| } else if string(deviceId) == "" { | |
| return errors.New("device_id is not yet present in ServerProvidedDataStore") | |
| } | |
| if err != nil { | |
| return err | |
| } | |
| if string(deviceId) == "" { | |
| return errors.New("device_id is not yet present in ServerProvidedDataStore") | |
| } |
There was a problem hiding this comment.
Shouldn't return the error. If we don't have that data, skip and and try the next piece of data. (Probably log the error)
| if err != nil { | ||
| return err | ||
| } else if string(organizationId) == "" { | ||
| return errors.New("organization_id is not yet present in ServerProvidedDataStore") | ||
| } |
There was a problem hiding this comment.
same
| if err != nil { | |
| return err | |
| } else if string(organizationId) == "" { | |
| return errors.New("organization_id is not yet present in ServerProvidedDataStore") | |
| } | |
| if err != nil { | |
| return err | |
| } | |
| if string(organizationId) == "" { | |
| return errors.New("organization_id is not yet present in ServerProvidedDataStore") | |
| } |
| if err != nil { | ||
| return err | ||
| } else if string(munemo) == "" { | ||
| return errors.New("munemo is not yet present in ServerProvidedDataStore") | ||
| } |
There was a problem hiding this comment.
same ... starting to feel like this should be a little helper func
| if err != nil { | |
| return err | |
| } else if string(munemo) == "" { | |
| return errors.New("munemo is not yet present in ServerProvidedDataStore") | |
| } | |
| if err != nil { | |
| return err | |
| } | |
| if string(munemo) == "" { | |
| return errors.New("munemo is not yet present in ServerProvidedDataStore") | |
| } |
There was a problem hiding this comment.
agreed, I will pull these into a helper. thank you!
| // subsystem. whenever new data is received, it will rewrite the metadata.json | ||
| // and metadata.plist files to our root install directory | ||
| metadataWriter struct { | ||
| ctx context.Context |
There was a problem hiding this comment.
context should not be embedded in structs. (and probably isn't needed)
| if err != nil { | ||
| return err | ||
| } else if string(deviceId) == "" { | ||
| return errors.New("device_id is not yet present in ServerProvidedDataStore") | ||
| } |
There was a problem hiding this comment.
Shouldn't return the error. If we don't have that data, skip and and try the next piece of data. (Probably log the error)
Co-authored-by: seph <seph@kolide.co>
Co-authored-by: seph <seph@kolide.co>
addresses #1404