Skip to content

Commit

Permalink
Merge pull request #246 from NetApp/236-bug-unable-to-set-cifs-acl-1
Browse files Browse the repository at this point in the history
fix set acl error.
  • Loading branch information
carchi8py authored Aug 7, 2024
2 parents 4eca6ef + 1f2b591 commit 5b54beb
Show file tree
Hide file tree
Showing 5 changed files with 376 additions and 16 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
## 1.1.3

BUG FIXES:

* **netapp-ontap_protocols_cifs_service_resource**: fixed on attribute checking ([#250](https://github.com/NetApp/terraform-provider-netapp-ontap/issues/250))
* **netapp-ontap_protocols_cifs_share_resource** :`acls` unable to update acls ([#236](https://github.com/NetApp/terraform-provider-netapp-ontap/issues/236))
* **netapp-ontap_protocols_san_igroups_resource**: fixed bug nil pointer dereference ([#247](https://github.com/NetApp/terraform-provider-netapp-ontap/issues/247))


Expand Down
26 changes: 13 additions & 13 deletions internal/interfaces/protocols_cifs_share.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,20 +52,20 @@ type ProtocolsCIFSShareResourceBodyDataModelONTAP struct {
Name string `mapstructure:"name,omitempty"` // can't be present in update, so omit empty.
SVM svm `mapstructure:"svm"`
Acls []Acls `mapstructure:"acls,omitempty"` // API complains if this is not omit empty
ChangeNotify bool `mapstructure:"change_notify"`
ChangeNotify bool `mapstructure:"change_notify,omitempty"`
Comment string `mapstructure:"comment,omitempty"` // API complains if this is not omit empty
ContinuouslyAvailable bool `mapstructure:"continuously_available"`
DirUmask int64 `mapstructure:"dir_umask"`
Encryption bool `mapstructure:"encryption"`
FileUmask int64 `mapstructure:"file_umask"`
ForceGroupForCreate string `mapstructure:"force_group_for_create"`
ContinuouslyAvailable bool `mapstructure:"continuously_available,omitempty"`
DirUmask int64 `mapstructure:"dir_umask,omitempty"`
Encryption bool `mapstructure:"encryption,omitempty"`
FileUmask int64 `mapstructure:"file_umask,omitempty"`
ForceGroupForCreate string `mapstructure:"force_group_for_create,omitempty"`
HomeDirectory bool `mapstructure:"home_directory,omitempty"` // can't be present in update, so omit empty.
NamespaceCaching bool `mapstructure:"namespace_caching"`
NoStrictSecurity bool `mapstructure:"no_strict_security"`
NamespaceCaching bool `mapstructure:"namespace_caching,omitempty"`
NoStrictSecurity bool `mapstructure:"no_strict_security,omitempty"`
OfflineFiles string `mapstructure:"offline_files,omitempty"` // API complains if this is not omit empty
Oplocks bool `mapstructure:"oplocks"`
Oplocks bool `mapstructure:"oplocks,omitempty"`
Path string `mapstructure:"path,omitempty"` // can't be present in update, so omit empty.
ShowSnapshot bool `mapstructure:"show_snapshot"`
ShowSnapshot bool `mapstructure:"show_snapshot,omitempty"`
UnixSymlink string `mapstructure:"unix_symlink,omitempty"` // API complains if this is not omit empty
VscanProfile string `mapstructure:"vscan_profile,omitempty"` // API complains if this is not omit empty
}
Expand Down Expand Up @@ -159,14 +159,14 @@ func CreateProtocolsCIFSShare(errorHandler *utils.ErrorHandler, r restclient.Res

// UpdateProtocolsCIFSShare to update protocols_cifs_share
func UpdateProtocolsCIFSShare(errorHandler *utils.ErrorHandler, r restclient.RestClient, body ProtocolsCIFSShareResourceBodyDataModelONTAP, name string, svmUUID string) error {
api := "/protocols/cifs/shares/"
api := fmt.Sprintf("/protocols/cifs/shares/%s/%s", svmUUID, name)
var bodyMap map[string]interface{}
if err := mapstructure.Decode(body, &bodyMap); err != nil {
return errorHandler.MakeAndReportError("error encoding protocols_cifs_share body", fmt.Sprintf("error on encoding %s body: %s, body: %#v", api, err, body))
}
statusCode, _, err := r.CallUpdateMethod(api+"/"+svmUUID+"/"+name, nil, bodyMap)
statusCode, _, err := r.CallUpdateMethod(api, nil, bodyMap)
if err != nil {
return errorHandler.MakeAndReportError("error updating protocols_cifs_share", fmt.Sprintf("error on POST %s: %s, statusCode %d", api, err, statusCode))
return errorHandler.MakeAndReportError("error updating protocols_cifs_share", fmt.Sprintf("error on PATCH %s: %s, statusCode %d", api, err, statusCode))
}
return nil
}
Expand Down
143 changes: 143 additions & 0 deletions internal/interfaces/protocols_cifs_share_acl.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
package interfaces

import (
"fmt"

"github.com/hashicorp/terraform-plugin-log/tflog"
"github.com/mitchellh/mapstructure"
"github.com/netapp/terraform-provider-netapp-ontap/internal/restclient"
"github.com/netapp/terraform-provider-netapp-ontap/internal/utils"
)

// ProtocolsCIFSShareACLGetDataModelONTAP describes the GET record data model using go types for mapping.
type ProtocolsCIFSShareACLGetDataModelONTAP struct {
Name string `mapstructure:"name"`
UUID string `mapstructure:"uuid"`
UserOrGroup string `mapstructure:"user_or_group"`
}

// ProtocolsCIFSShareACLResourceBodyDataModelONTAP describes the body data model using go types for mapping.
type ProtocolsCIFSShareACLResourceBodyDataModelONTAP struct {
// Name string `mapstructure:"name"`
// SVM svm `mapstructure:"svm"`
Permission string `mapstructure:"permission"`
UserOrGroup string `mapstructure:"user_or_group"`
Type string `mapstructure:"type"`
}

// ProtocolsCIFSShareACLDataSourceFilterModel describes the data source data model for queries.
type ProtocolsCIFSShareACLDataSourceFilterModel struct {
Name string `mapstructure:"name"`
SVMName string `mapstructure:"svm.name"`
UserOrGroup string `mapstructure:"user_or_group"`
}

// GetProtocolsCIFSShareACLByName to get protocols_cifs_share_acl info
func GetProtocolsCIFSShareACLByName(errorHandler *utils.ErrorHandler, r restclient.RestClient, name string, svmName string) (*ProtocolsCIFSShareACLGetDataModelONTAP, error) {
api := "api_url"
query := r.NewQuery()
query.Set("name", name)
if svmName == "" {
query.Set("scope", "cluster")
} else {
query.Set("svm.name", svmName)
query.Set("scope", "svm")
}
query.Fields([]string{"name", "svm.name", "ip", "scope"})
statusCode, response, err := r.GetNilOrOneRecord(api, query, nil)
if err == nil && response == nil {
err = fmt.Errorf("no response for GET %s", api)
}
if err != nil {
return nil, errorHandler.MakeAndReportError("error reading protocols_cifs_share_acl info", fmt.Sprintf("error on GET %s: %s, statusCode %d", api, err, statusCode))
}

var dataONTAP ProtocolsCIFSShareACLGetDataModelONTAP
if err := mapstructure.Decode(response, &dataONTAP); err != nil {
return nil, errorHandler.MakeAndReportError(fmt.Sprintf("failed to decode response from GET %s", api),
fmt.Sprintf("error: %s, statusCode %d, response %#v", err, statusCode, response))
}
tflog.Debug(errorHandler.Ctx, fmt.Sprintf("Read protocols_cifs_share_acl data source: %#v", dataONTAP))
return &dataONTAP, nil
}

// GetProtocolsCIFSShareAcls to get protocols_cifs_share_acl info for all resources matching a filter
func GetProtocolsCIFSShareAcls(errorHandler *utils.ErrorHandler, r restclient.RestClient, filter *ProtocolsCIFSShareACLDataSourceFilterModel, svmName string, shareName string) ([]ProtocolsCIFSShareACLGetDataModelONTAP, error) {
api := fmt.Sprintf("/protocols/cifs/shares/%s/%s/acls", svmName, shareName)
query := r.NewQuery()
query.Fields([]string{"name", "svm.name", "scope"})
if filter != nil {
var filterMap map[string]interface{}
if err := mapstructure.Decode(filter, &filterMap); err != nil {
return nil, errorHandler.MakeAndReportError("error encoding protocols_cifs_share_acls filter info", fmt.Sprintf("error on filter %#v: %s", filter, err))
}
query.SetValues(filterMap)
}
statusCode, response, err := r.GetZeroOrMoreRecords(api, query, nil)
if err == nil && response == nil {
err = fmt.Errorf("no response for GET %s", api)
}
if err != nil {
return nil, errorHandler.MakeAndReportError("error reading protocols_cifs_share_acls info", fmt.Sprintf("error on GET %s: %s, statusCode %d", api, err, statusCode))
}

var dataONTAP []ProtocolsCIFSShareACLGetDataModelONTAP
for _, info := range response {
var record ProtocolsCIFSShareACLGetDataModelONTAP
if err := mapstructure.Decode(info, &record); err != nil {
return nil, errorHandler.MakeAndReportError(fmt.Sprintf("failed to decode response from GET %s", api),
fmt.Sprintf("error: %s, statusCode %d, info %#v", err, statusCode, info))
}
dataONTAP = append(dataONTAP, record)
}
tflog.Debug(errorHandler.Ctx, fmt.Sprintf("Read protocols_cifs_share_acls data source: %#v", dataONTAP))
return dataONTAP, nil
}

// CreateProtocolsCIFSShareACL to create protocols_cifs_share_acl
func CreateProtocolsCIFSShareACL(errorHandler *utils.ErrorHandler, r restclient.RestClient, body ProtocolsCIFSShareACLResourceBodyDataModelONTAP, svmID string, shareName string) (*ProtocolsCIFSShareACLGetDataModelONTAP, error) {
api := fmt.Sprintf("/protocols/cifs/shares/%s/%s/acls", svmID, shareName)
var bodyMap map[string]interface{}
if err := mapstructure.Decode(body, &bodyMap); err != nil {
return nil, errorHandler.MakeAndReportError("error encoding protocols_cifs_share_acl body", fmt.Sprintf("error on encoding %s body: %s, body: %#v", api, err, body))
}
query := r.NewQuery()
query.Add("return_records", "true")
statusCode, response, err := r.CallCreateMethod(api, query, bodyMap)
if err != nil {
return nil, errorHandler.MakeAndReportError("error creating protocols_cifs_share_acl", fmt.Sprintf("error on POST %s: %s, statusCode %d", api, err, statusCode))
}

var dataONTAP ProtocolsCIFSShareACLGetDataModelONTAP
if err := mapstructure.Decode(response.Records[0], &dataONTAP); err != nil {
return nil, errorHandler.MakeAndReportError("error decoding protocols_cifs_share_acl info", fmt.Sprintf("error on decode storage/protocols_cifs_share_acls info: %s, statusCode %d, response %#v", err, statusCode, response))
}
tflog.Debug(errorHandler.Ctx, fmt.Sprintf("Create protocols_cifs_share_acl source - udata: %#v", dataONTAP))
return &dataONTAP, nil
}

// UpdateProtocolsCIFSShareACL to update protocols_cifs_share_acl
func UpdateProtocolsCIFSShareACL(errorHandler *utils.ErrorHandler, r restclient.RestClient, body ProtocolsCIFSShareACLResourceBodyDataModelONTAP, svmID string, shareName string, userOrGroup string, aclType string) error {
api := fmt.Sprintf("/protocols/cifs/shares/%s/%s/acls/%s/%s", svmID, shareName, userOrGroup, aclType)
var bodyMap map[string]interface{}
if err := mapstructure.Decode(body, &bodyMap); err != nil {
return errorHandler.MakeAndReportError("error encoding protocols_cifs_share_acl body", fmt.Sprintf("error on encoding %s body: %s, body: %#v", api, err, body))
}
delete(bodyMap, "type") // type is not returned in the response
delete(bodyMap, "user_or_group") // user_or_group is not returned in the response
statusCode, _, err := r.CallUpdateMethod(api, nil, bodyMap)
if err != nil {
return errorHandler.MakeAndReportError("error updating protocols_cifs_share_acl", fmt.Sprintf("error on PATCH %s: %s, statusCode %d", api, err, statusCode))
}
return nil
}

// DeleteProtocolsCIFSShareACL to delete protocols_cifs_share_acl
func DeleteProtocolsCIFSShareACL(errorHandler *utils.ErrorHandler, r restclient.RestClient, svmID string, shareName string, userOrGroup string, aclType string) error {
api := fmt.Sprintf("/protocols/cifs/shares/%s/%s/acls/%s/%s", svmID, shareName, userOrGroup, aclType)
statusCode, _, err := r.CallDeleteMethod(api, nil, nil)
if err != nil {
return errorHandler.MakeAndReportError("error deleting protocols_cifs_share_acl", fmt.Sprintf("error on DELETE %s: %s, statusCode %d", api, err, statusCode))
}
return nil
}
123 changes: 122 additions & 1 deletion internal/provider/protocols_cifs_share_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,7 @@ func (r *ProtocolsCIFSShareResource) Create(ctx context.Context, req resource.Cr
body.SVM.Name = data.SVMName.ValueString()
body.Path = data.Path.ValueString()

configHasDefaultACL := false
if !data.Acls.IsUnknown() {
aclsList := []interfaces.Acls{}
elements := make([]types.Object, 0, len(data.Acls.Elements()))
Expand All @@ -451,7 +452,9 @@ func (r *ProtocolsCIFSShareResource) Create(ctx context.Context, req resource.Cr
interfacesAcls.Permission = acls.Permission
interfacesAcls.Type = acls.Type
interfacesAcls.UserOrGroup = acls.UserOrGroup

if acls.UserOrGroup == "Everyone" && acls.Permission == "full_control" {
configHasDefaultACL = true
}
aclsList = append(aclsList, interfacesAcls)
}
body.Acls = aclsList
Expand Down Expand Up @@ -553,6 +556,20 @@ func (r *ProtocolsCIFSShareResource) Create(ctx context.Context, req resource.Cr
data.Acls = setValue
} else {
for _, acls := range restInfo.Acls {
//If the config file does not have acl set user_or_group as "Everyone / Full Control", the API will create one by default. Need to delete it if user does not want one.
if acls.UserOrGroup == "Everyone" && acls.Permission == "full_control" && !configHasDefaultACL {
svm, err := interfaces.GetSvmByName(errorHandler, *client, data.SVMName.ValueString())
if err != nil {
return
}
err = interfaces.DeleteProtocolsCIFSShareACL(errorHandler, *client, svm.UUID, data.Name.ValueString(), acls.UserOrGroup, acls.Type)
if err != nil {
// error reporting done inside DeleteProtocolsCIFSShareAcl
return
}
continue
}

elementType := map[string]attr.Type{
"permission": types.StringType,
"type": types.StringType,
Expand Down Expand Up @@ -691,11 +708,115 @@ func (r *ProtocolsCIFSShareResource) Update(ctx context.Context, req resource.Up
}
}

if !plan.ContinuouslyAvailable.IsUnknown() {
if plan.ContinuouslyAvailable != state.ContinuouslyAvailable {
body.ContinuouslyAvailable = plan.ContinuouslyAvailable.ValueBool()
}
}

svm, err := interfaces.GetSvmByName(errorHandler, *client, plan.SVMName.ValueString())
if err != nil {
return
}

// have no luck updating acls using PATCH cifs/shares API sucessfully, so we have to use the acls set of API.
if !plan.Acls.IsUnknown() {
// reading acls from plan
planeAcls := make([]types.Object, 0, len(plan.Acls.Elements()))
diags := plan.Acls.ElementsAs(ctx, &planeAcls, false)
if diags.HasError() {
resp.Diagnostics.Append(diags...)
return
}
// reading acls from state
stateAcls := make([]types.Object, 0, len(state.Acls.Elements()))
diags = state.Acls.ElementsAs(ctx, &stateAcls, false)
if diags.HasError() {
resp.Diagnostics.Append(diags...)
return
}
// iterate over plan acls and compare with state acls. Make create or update or delete calls accordingly.
for _, element := range stateAcls {
var stateACLElement ProtocolsCIFSShareResourceAcls
diags := element.As(ctx, &stateACLElement, basetypes.ObjectAsOptions{})
if diags.HasError() {
resp.Diagnostics.Append(diags...)
return
}
for index, planACL := range planeAcls {
var planACLElement ProtocolsCIFSShareResourceAcls
diags := planACL.As(ctx, &planACLElement, basetypes.ObjectAsOptions{})
if diags.HasError() {
resp.Diagnostics.Append(diags...)
return
}
// if 'userOrGroup' and 'type' matches, then we know it's not a create action. If permission is same, then break the loop because nothing to update.
if stateACLElement.UserOrGroup == planACLElement.UserOrGroup && stateACLElement.Type == planACLElement.Type {
if stateACLElement.Permission == planACLElement.Permission {
break
} else {
// update the acls since permission is different
interfacesAcls := interfaces.ProtocolsCIFSShareACLResourceBodyDataModelONTAP{}
interfacesAcls.Permission = planACLElement.Permission
err = interfaces.UpdateProtocolsCIFSShareACL(errorHandler, *client, interfacesAcls, svm.UUID, plan.Name.ValueString(), planACLElement.UserOrGroup, planACLElement.Type)
if err != nil {
return
}
break
}
}
// if we reach the end of stateAcls, then we know it's a delete action because it was not found in plan acls.
if index == len(planeAcls)-1 {
err = interfaces.DeleteProtocolsCIFSShareACL(errorHandler, *client, svm.UUID, plan.Name.ValueString(), stateACLElement.UserOrGroup, stateACLElement.Type)
if err != nil {
return
}

}
}

}
// now handle create action
for _, planACL := range planeAcls {
var planACLElement ProtocolsCIFSShareResourceAcls
diags := planACL.As(ctx, &planACLElement, basetypes.ObjectAsOptions{})
if diags.HasError() {
resp.Diagnostics.Append(diags...)
return
}

for index, element := range stateAcls {
var stateACLElement ProtocolsCIFSShareResourceAcls
diags := element.As(ctx, &stateACLElement, basetypes.ObjectAsOptions{})
if diags.HasError() {
resp.Diagnostics.Append(diags...)
return
}
if stateACLElement.UserOrGroup == planACLElement.UserOrGroup && stateACLElement.Type == planACLElement.Type {
if stateACLElement.Permission == planACLElement.Permission {
break
} else {
// update is already handled by above logic, so break
break
}
}
// if we reach the end of planAcls, then we know it's a create action because it was not found in state acls.
if index == len(stateAcls)-1 {
interfacesAcls := interfaces.ProtocolsCIFSShareACLResourceBodyDataModelONTAP{}
interfacesAcls.Permission = planACLElement.Permission
interfacesAcls.Type = planACLElement.Type
interfacesAcls.UserOrGroup = planACLElement.UserOrGroup
_, err = interfaces.CreateProtocolsCIFSShareACL(errorHandler, *client, interfacesAcls, svm.UUID, plan.Name.ValueString())
if err != nil {
return
}
}
}

}

}

err = interfaces.UpdateProtocolsCIFSShare(errorHandler, *client, body, plan.Name.ValueString(), svm.UUID)
if err != nil {
return
Expand Down
Loading

0 comments on commit 5b54beb

Please sign in to comment.