-
Notifications
You must be signed in to change notification settings - Fork 10
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.
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
Fixed configs. Currently working locally. #25
Conversation
8779aa2
to
41270f2
Compare
webhook_settings: | ||
failing_policies_webhook: | ||
enable_failing_policies_webhook: true | ||
destination_url: https://example.tines.com/webhook | ||
policy_ids: [1, 2, 3, 4, 5,6 ,7, 8, 9] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we pulling these out to simplify the example? Or, did we cut webhook_settings
functionality?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Our current fleetctl apply
does not support team webhook_settings or integrations, right? And I'm using the existing APIs, so fleetctl gitops
doesn't support these either without additional work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fleetctl apply
does not support teamwebhook_settings
orintegrations
, right?
@getvictor I'm not positive.
@mna when you get the chance, can you please sanity check us here?
If this is the case, it makes sense to punt on adding support for webhook_settings
and integrations
. We can add support for this in fleetctl gitops
later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAICT that's correct, no webhook or integrations can be set via apply team specs: https://github.com/fleetdm/fleet/blob/main/server/fleet/teams.go#L387
queries: | ||
- path: ./lib/collect-fleetd-update-channel.queries.yml | ||
- path: ./lib/collect-fleetd-update-channels.queries.yml |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to to add an indent here before each query? (- path
) I see you added indents for custom_settings
.
Or, should the custom_settings
items not have an indent?
- path: ../lib/macos-device-health.policies.yml | ||
- path: ../lib/windows-device-health.policies.yml |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar comment about indentation here. Just want to make sure we're being consistent w/ other options that accept arrays (custom_settings
, scripts
, etc.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dropped some comments about indentations.
Otherwise, looks good to me!
I'll review the indentation before submitting the next PR. |
Did some cleanup of the configs. These configs currently work with my local implementation.