Skip to content
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

Fix regex to handle all possible IP addresses #1489

Merged
merged 1 commit into from Jan 29, 2020
Merged

Fix regex to handle all possible IP addresses #1489

merged 1 commit into from Jan 29, 2020

Conversation

@smlx
Copy link
Contributor

smlx commented Dec 12, 2019

Checklist

  • Affected Issues have been mentioned in the Closing issues section
  • Documentation has been written/updated.
  • Changelog entry has been written

If you run make minishift more than once then the regex may not match if the last octet of the minishift IP doesn't have 3 digits. This PR updates the regex to handle all possible IPs.

Changelog Entry

Improvement - increase robustness of build scripts for lagoon development environment

Closing issues

n/a

@Schnitzel Schnitzel modified the milestones: v1.2.0, v1.3.0 Dec 13, 2019
@Schnitzel

This comment has been minimized.

Copy link
Member

Schnitzel commented Dec 16, 2019

@smlx I don't think that is correct, the IP that is in all files that should be replaced is 192.168.42.100, for example here: https://github.com/amazeeio/lagoon/blob/master/docker-compose.yaml#L235

But your regex now also matches 192.168.42.1 which should not be replaced, example: https://github.com/amazeeio/lagoon/blob/master/docker-compose.yaml#L287 and so the tests fail.

can you update the regex so it matches, 192.168\.[0-9]{1,3}\.[0-9]{1,3} but NOT 192.168.42.1 ?

@smlx

This comment has been minimized.

Copy link
Contributor Author

smlx commented Dec 17, 2019

ah, yeah that's a problem. I'll fix it up.

@smlx smlx force-pushed the fix-ip-regex branch from af17d7f to bcb31cf Dec 17, 2019
@smlx

This comment has been minimized.

Copy link
Contributor Author

smlx commented Dec 17, 2019

ok, should be good to go

@smlx smlx force-pushed the fix-ip-regex branch from bcb31cf to 81aaaf6 Jan 8, 2020
@smlx smlx force-pushed the fix-ip-regex branch from 81aaaf6 to 424b273 Jan 20, 2020
@smlx smlx requested a review from Schnitzel Jan 20, 2020
Avoid .1 as that is the host gateway.
@smlx smlx force-pushed the fix-ip-regex branch from 424b273 to c313d7c Jan 22, 2020
@smlx smlx mentioned this pull request Jan 22, 2020
2 of 3 tasks complete
@Schnitzel Schnitzel merged commit 169cb34 into master Jan 29, 2020
1 check failed
1 check failed
continuous-integration/jenkins/pr-merge This commit cannot be built
Details
@shreddedbacon

This comment has been minimized.

Copy link
Member

shreddedbacon commented Feb 3, 2020

@smlx this is broken with darwin :(
The version of sed installed doesn't support the regex pattern properly.

$ sed --version
sed: illegal option -- -
usage: sed script [-Ealn] [-i extension] [file ...]
       sed [-Ealn] [-i extension] [-e script] ... [-f script_file] ... [file ...]

The -E flag seems to support regex rather than -e

-E      Interpret regular expressions as extended (modern) regular expressions rather than basic regular expressions (BRE's).  The re_format(7) manual page fully describes both formats.
-e command
             Append the editing commands specified by the command argument to the list of commands.

So this works when run outside of make

sed -i '' -E "s/192.168\.[0-9]{1,3}\.([2-9]|[0-9]{2,3})/192.168.42.100/g" local-dev/api-data/01-populate-api-data.gql docker-compose.yaml

Changing the make file to use -E fails though, possibly the escaping breaks it?

sed -i '' -E "s/192.168\.[0-9]\{1,3\}\.\([2-9]\|[0-9]\{2,3\}\)/$${OPENSHIFT_MACHINE_IP}/g" local-dev/api-data/01-populate-api-data.gql docker-compose.yaml;

As it works when not escaped like this in the make file

sed -i '' -E "s/192.168\.[0-9]{1,3}\.([2-9]|[0-9]{2,3})/$${OPENSHIFT_MACHINE_IP}/g" local-dev/api-data/01-populate-api-data.gql docker-compose.yaml;
@smlx

This comment has been minimized.

Copy link
Contributor Author

smlx commented Feb 4, 2020

@shreddedbacon sorry about that D: could you fix it up in a new PR?

@smlx smlx deleted the fix-ip-regex branch Feb 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.