CLOUDP-78181: ip access list (part 2) #91
Conversation
…to main � Conflicts: � README.md
| IPAddress string `json:"ipAddress,omitempty"` | ||
| } | ||
|
|
||
| // ToAtlas converts the ProjectIPAccessList to native Atlas client format. |
There was a problem hiding this comment.
just moved these two methods below the ones for AtlasProject
| // so we have to list all possible formats | ||
| func ParseISO8601(dateTime string) (time.Time, error) { | ||
| return time.Parse(dateTime, "2006-01-02T15:04:05-0700") | ||
| parse, err := time.Parse("2006-01-02T15:04:05-07", dateTime) |
There was a problem hiding this comment.
[q] Do we really need all of these? As per the docs, Atlas will always use UTC for Access Lists & DB Users
There was a problem hiding this comment.
nope, the docs say about "Timestamp in ISO 8601" and in practice this means different variations including the time zones
There was a problem hiding this comment.
I still think Atlas returns UTC only, but thinking about it - this might be useful in the future if we decide to change/validate user input somehow since user input can contain a timezone shift.
There was a problem hiding this comment.
yes, Atlas returns UTC only but accepts any valid ISO 8601 date - so I tried to cover all possible formats :)
…nto CLOUDP-78181-ip-access-list-2 # Conflicts: # pkg/controller/workflow/reason.go
| allSpecified := isNotEmpty(list.AwsSecurityGroup) && isNotEmpty(list.CIDRBlock) && isNotEmpty(list.IPAddress) | ||
| if !onlyOneSpecified || allSpecified { | ||
| return errors.New("only one of the 'awsSecurityGroup', 'cidrBlock' or 'ipAddress' is required be specified") | ||
| } |
There was a problem hiding this comment.
[nit] May I suggest we replace this with a function? I know this is a one-time special thing, but this is really hard to read.
Maybe something like this: https://goplay.space/#889tv6pZ7l-
| // so we have to list all possible formats | ||
| func ParseISO8601(dateTime string) (time.Time, error) { | ||
| return time.Parse(dateTime, "2006-01-02T15:04:05-0700") | ||
| parse, err := time.Parse("2006-01-02T15:04:05-07", dateTime) |
There was a problem hiding this comment.
I still think Atlas returns UTC only, but thinking about it - this might be useful in the future if we decide to change/validate user input somehow since user input can contain a timezone shift.
(This PR should be reviewed after the #90)
This adds more fixes and tests for project IP access list (especially for the date field as it wasn't trivial to parse ISO8601)
This doesn't handle the "expired" lists though - will be addressed in the next PR