-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Adds edit support for IC plugin to tctl edit
#52243
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1618,11 +1618,15 @@ func (p *pluginResourceWrapper) UnmarshalJSON(data []byte) error { | |
settingsEntraID = "entra_id" | ||
settingsDatadogIncidentManagement = "datadog_incident_management" | ||
settingsEmailAccessPlugin = "email_access_plugin" | ||
settingsAWSIdentityCenter = "aws_ic" | ||
) | ||
type unknownPluginType struct { | ||
Spec struct { | ||
Settings map[string]json.RawMessage `json:"Settings"` | ||
} `json:"spec"` | ||
Status struct { | ||
Details map[string]json.RawMessage `json:"Details"` | ||
} `json:"status"` | ||
Credentials struct { | ||
Credentials map[string]json.RawMessage `json:"Credentials"` | ||
} `json:"credentials"` | ||
|
@@ -1658,8 +1662,7 @@ func (p *pluginResourceWrapper) UnmarshalJSON(data []byte) error { | |
} | ||
} | ||
|
||
for k := range unknownPlugin.Spec.Settings { | ||
|
||
for k, value := range unknownPlugin.Spec.Settings { | ||
switch k { | ||
case settingsSlackAccessPlugin: | ||
p.PluginV1.Spec.Settings = &types.PluginSpecV1_SlackAccessPlugin{} | ||
|
@@ -1689,17 +1692,99 @@ func (p *pluginResourceWrapper) UnmarshalJSON(data []byte) error { | |
p.PluginV1.Spec.Settings = &types.PluginSpecV1_Datadog{} | ||
case settingsEmailAccessPlugin: | ||
p.PluginV1.Spec.Settings = &types.PluginSpecV1_Email{} | ||
case settingsAWSIdentityCenter: | ||
settings := &types.PluginSpecV1_AwsIc{ | ||
AwsIc: &types.PluginAWSICSettings{}, | ||
} | ||
p.PluginV1.Spec.Settings = settings | ||
|
||
unmshallingWrapper := icSettingsWrapper{inner: settings.AwsIc} | ||
if err := json.Unmarshal(value, &unmshallingWrapper); err != nil { | ||
return trace.Wrap(err) | ||
} | ||
default: | ||
return trace.BadParameter("unsupported plugin type: %v", k) | ||
} | ||
} | ||
|
||
if len(unknownPlugin.Status.Details) > 1 { | ||
return trace.BadParameter("malformed status details") | ||
} | ||
for k := range unknownPlugin.Status.Details { | ||
switch k { | ||
case settingsAWSIdentityCenter: | ||
p.PluginV1.Status.Details = &types.PluginStatusV1_AwsIc{} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does reseting plugin details is safe ? Does the Backend will allow overwrite this settings ? Could you add a comment answering "Why" the PluginV1.Status.Details field is reseted for settingsAWSIdentityCenter plugin ? |
||
} | ||
} | ||
|
||
if err := json.Unmarshal(data, &p.PluginV1); err != nil { | ||
return err | ||
} | ||
return nil | ||
} | ||
|
||
// icSettingsWrapper is a wrapper around the Identity Center plugin settings to | ||
// provide custom unmarshalling. | ||
type icSettingsWrapper struct { | ||
inner *types.PluginAWSICSettings | ||
} | ||
|
||
// UnmarshalJSON implements custom JSON-unmarshaling for the Identity Center | ||
// plugin settings. This custom unmarshaler is required to unpack the structure | ||
// of the polymorphic filters in the plugin settings, which otherise cannot be | ||
// unpacked. | ||
func (s *icSettingsWrapper) UnmarshalJSON(data []byte) error { | ||
type resourceFilter struct { | ||
Include map[string]json.RawMessage `json:"Include"` | ||
} | ||
|
||
var settings struct { | ||
AccountFilters []resourceFilter `json:"aws_accounts_filters"` | ||
GroupFilters []resourceFilter `json:"group_sync_filters"` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The current AWS IC flow does not allow updating groups after the AWS IC plugin is enrolled. Groups are fetched only once during plugin installation, and even if a filter is updated afterward, the plugin skips the group sync.
So if we want to proceed with this change:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So, if i'm reading you correctly, in the case of a changed group filters we would basically want to re-run the import? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, we discussed this in a 1vs1. The final update implementation should ensure that the backend allows updating GroupFilters once the initial AWS enrollment is completed. Since this is not directly related to the tctl update flow, the backend alignment can be handled in a separate PR. However, we need to ensure that if we advise customers to use tctl edit plugin aws, the backend flow properly supports updating GroupFilters |
||
} | ||
|
||
// unpackFilters only creates the structure of the filters so that the | ||
// normal JSON unmarshaller knows how to fill in the actual values | ||
unpackFilters := func(src []resourceFilter) ([]*types.AWSICResourceFilter, error) { | ||
var dst []*types.AWSICResourceFilter | ||
for _, f := range src { | ||
if len(f.Include) != 1 { | ||
return nil, trace.BadParameter("Malformed filter") | ||
} | ||
for k := range f.Include { | ||
switch k { | ||
case "id": | ||
dst = append(dst, &types.AWSICResourceFilter{Include: &types.AWSICResourceFilter_Id{}}) | ||
|
||
case "name_regex": | ||
dst = append(dst, &types.AWSICResourceFilter{Include: &types.AWSICResourceFilter_NameRegex{}}) | ||
|
||
default: | ||
return nil, trace.BadParameter("Unexpected filter key: %s", k) | ||
} | ||
} | ||
} | ||
return dst, nil | ||
} | ||
|
||
if err := json.Unmarshal(data, &settings); err != nil { | ||
return trace.Wrap(err) | ||
} | ||
|
||
var err error | ||
s.inner.AwsAccountsFilters, err = unpackFilters(settings.AccountFilters) | ||
if err != nil { | ||
return trace.Wrap(err) | ||
} | ||
|
||
s.inner.GroupSyncFilters, err = unpackFilters(settings.GroupFilters) | ||
if err != nil { | ||
return trace.Wrap(err) | ||
} | ||
|
||
return nil | ||
} | ||
|
||
func (c *pluginCollection) resources() []types.Resource { | ||
r := make([]types.Resource, len(c.plugins)) | ||
for i, resource := range c.plugins { | ||
|
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.
Can we use a proto jsonpb package where oneof marhsling/unmershaling is already implemented ?