Skip to content

Jamf config for PluginSpecV1#26374

Merged
flyinghermit merged 7 commits intomasterfrom
sshah/mdm-plugin-proto
May 19, 2023
Merged

Jamf config for PluginSpecV1#26374
flyinghermit merged 7 commits intomasterfrom
sshah/mdm-plugin-proto

Conversation

@flyinghermit
Copy link
Copy Markdown
Contributor

@flyinghermit flyinghermit commented May 16, 2023

  • PluginJamfSettings added to PluginSpecV1.
  • Updated PluginV1.GetType() to handle jamf type.

@flyinghermit flyinghermit changed the title Jamf specific config added to PluginsV1 Jamf config for PluginSpecV1 May 18, 2023
@flyinghermit flyinghermit marked this pull request as ready for review May 18, 2023 13:09
@github-actions github-actions Bot requested review from camscale and jakule May 18, 2023 13:09
Comment thread api/types/plugin.go Outdated
// PluginTypeOkta is the Okta plugin
PluginTypeOkta = "okta"
// PluginTypeMDM represents MDM plugin type collectively
PluginTypeMDM = "mdm"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This looks unused

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This will be used in the Web API to differentiate MDM plugins vs Access plugins.

Copy link
Copy Markdown
Contributor

@justinas justinas May 19, 2023

Choose a reason for hiding this comment

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

Plugin can only have one type, so would Jamf have type mdm or jamf?

What are the expected uses of grouping different plugins as mdm? We can probably tell if the plugin is MDM "statically", e.g.

func (p *PluginV1) IsMDM() bool {
    return p.GetType() == PluginTypeJamf || p.GetType() == PluginTypeSomeOtherMDM
}

We do not group Slack/Discord/MSTeams as some PluginTypeAccess. Well, one could argue that's because only Slack exists from that category for now 😅

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We can also have a label for MDM plugins, if that helps us query/filter them somehow.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

sorry I missed the input box here and replied below in comment thread below.

To add more, I think it will be way easy and less compute hungry to just compare string rather than the example above. I think metadata label should work as well but should those label's be used for customer specific labeling instead of internal system level labeling? I don't have strong opinion on that but just think PluginTypeMDM will be simple option to group mdm types.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

SubKind sounds okay, I guess.

Type is a bit different - it is specific to Plugin, is not a field (instead derived from the Settings variant), and is already used for concrete types (Slack, OpenAI, etc.), rather than categories (access, MDM, etc.)

I would suggest leaving PluginTypeJamf, and have MDM as the SubKind. Perhaps introduce PluginKindAccessRequest or similar, and PluginKindMDM.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hmm, that sounds more reasonable. I'll add a new PluginSubKind types and remove MDM type from here.

Comment thread api/types/plugin.go
}
}

// PluginIdSecretCredential can be OAuth2-like client_id and client_secret or username and password.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In what cases will we accept OAuth2 client_id and client_secret from the user? Will this be how we handle Mattermost etc., where we can not have one predefined Client ID, owned by us?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Self hosted Mattermost was in mind when I wrote that comment. But basically I tried to make this as generic as possible where we can recevice clientID:clientSecet, apiKey:apiToken, uname:pass.

Comment thread api/types/plugin.go Outdated
// PluginTypeOkta is the Okta plugin
PluginTypeOkta = "okta"
// PluginTypeMDM represents MDM plugin type collectively
PluginTypeMDM = "mdm"
Copy link
Copy Markdown
Contributor

@justinas justinas May 19, 2023

Choose a reason for hiding this comment

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

Plugin can only have one type, so would Jamf have type mdm or jamf?

What are the expected uses of grouping different plugins as mdm? We can probably tell if the plugin is MDM "statically", e.g.

func (p *PluginV1) IsMDM() bool {
    return p.GetType() == PluginTypeJamf || p.GetType() == PluginTypeSomeOtherMDM
}

We do not group Slack/Discord/MSTeams as some PluginTypeAccess. Well, one could argue that's because only Slack exists from that category for now 😅

@flyinghermit flyinghermit disabled auto-merge May 19, 2023 12:40
@flyinghermit
Copy link
Copy Markdown
Contributor Author

@justinas

Plugin can only have one type, so would Jamf have type mdm or jamf? What are the expected uses of grouping plugins as mdm? We can probably tell if the plugin is MDM "statically", e.g.

There's possibly two place where MDM type will be useful:

  1. Use the mdm type to filter generic mdm types in the web api. For example, we can have different createPlugin cases for MDM vs access plugins types. Example in this draft PR.

  2. Use the mdm type in PluginV1 Subkind so it can be handy to filter mdm resource types later.

@flyinghermit flyinghermit added this pull request to the merge queue May 19, 2023
Merged via the queue into master with commit 9bb5bcf May 19, 2023
@flyinghermit flyinghermit deleted the sshah/mdm-plugin-proto branch May 19, 2023 16:19
mdwn pushed a commit that referenced this pull request Jun 6, 2023
* jamf specefic config added to proto and plugin type

* JamfSpecV1 added to PluginJamfSetting

* remove PluginCredentialType type

* update plugin jamf spec name

* removed mdm from plugin type and added a new PluginSubkind
mdwn added a commit that referenced this pull request Jun 6, 2023
* jamf specefic config added to proto and plugin type

* JamfSpecV1 added to PluginJamfSetting

* remove PluginCredentialType type

* update plugin jamf spec name

* removed mdm from plugin type and added a new PluginSubkind

Co-authored-by: Sakshyam Shah <sshah@goteleport.com>
@r0mant r0mant mentioned this pull request Jul 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants