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

Added Slack role #205

Merged
merged 4 commits into from
Aug 21, 2019
Merged

Added Slack role #205

merged 4 commits into from
Aug 21, 2019

Conversation

kingoftheconnors
Copy link
Contributor

This is a role for using Matrix-Appservice-Slack.

There is no documentation yet, but to use it you need to define slack_control_room_id and matrix_appservice_slack_enabled in the inventory/host_vars/.../vars.yml file.

The --tags=setup-all and start work fine, but if you check the journalctl -fu matrix-appservice-slack you'll PROBABLY get a Unhandled rejection Error: connect ECONNREFUSED error. The matrix port connects to the homeserver itself, similar to the discord appservice and is right now set to 8008.

After I settle a few other projects, I hope to cycle back to this, but feel free to give it a whirl if you want!

@kingoftheconnors
Copy link
Contributor Author

Info about using this bridge further can be found at https://github.com/matrix-org/matrix-appservice-slack

It'll detail how to get the "control-room-id" (which is easy in Riot because you can just write any message in the control-room-channel and then use check-source) and how to invite the bot if you get that far.

Copy link
Owner

@spantaleev spantaleev left a comment

Choose a reason for hiding this comment

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

Thank you for starting on this! 👍

I haven't tested anything yet, but I've submitted some feedback based on what I saw from just reading the code.

I think it moves us in the right direction. Even if it doesn't completely make the bridge work, it should bring us to a point where we're almost there.

roles/matrix-bridge-appservice-slack/defaults/main.yml Outdated Show resolved Hide resolved
roles/matrix-bridge-appservice-slack/defaults/main.yml Outdated Show resolved Hide resolved
roles/matrix-bridge-appservice-slack/defaults/main.yml Outdated Show resolved Hide resolved
roles/matrix-bridge-appservice-slack/defaults/main.yml Outdated Show resolved Hide resolved
roles/matrix-bridge-appservice-slack/defaults/main.yml Outdated Show resolved Hide resolved
roles/matrix-bridge-appservice-slack/defaults/main.yml Outdated Show resolved Hide resolved
@Cadair
Copy link
Contributor

Cadair commented Jun 24, 2019

@kingoftheconnors @spantaleev Is there anything in here that could be made easier by changes in the slack AS itself? I am currently making a bunch of changes to it, so I could fold in any feedback you have.

One of the things I plan to add is making the path to the registration file configurable in the config.json file.

@kingoftheconnors
Copy link
Contributor Author

@kingoftheconnors @spantaleev Is there anything in here that could be made easier by changes in the slack AS itself? I am currently making a bunch of changes to it, so I could fold in any feedback you have.

One of the things I plan to add is making the path to the registration file configurable in the config.json file.

One thing that hurt organizationally was how the slack-registration file and databases had to be placed in a specific file together. If more freedom for where slack-registration and the database files could be found (such as saying "-d /data -f "/config" to signify looking for the databases in the /data file and the registration file in the /config file), that would help with organizing the files on the HS.

Also, the documentation could be better. The significance of 8008 and homeserverURL vs inboundURI is never explained.

@spantaleev
Copy link
Owner

Off the top of my head, I can't figure out why we need to pass -p MATRIX_PORT to app.js, when we've already specified the homeserver URL in the configuration file.

@Cadair
Copy link
Contributor

Cadair commented Jun 24, 2019

@spantaleev it's the port the appservice listens on for pushes from synapse where as the HS url is the other way around?

@kingoftheconnors Yeah, I will look into separating the config and the data, probably have the data locations specified in the config json.

@Cadair
Copy link
Contributor

Cadair commented Jun 24, 2019

So it turns out there is a dbdir: option in config.json which allows you to specify where the data goes. Looks like I didn't document it anywhere though 🤦‍♂️

@Cadair
Copy link
Contributor

Cadair commented Jun 25, 2019

I added config for the HS registration file in matrix-org/matrix-appservice-slack#143

@spantaleev
Copy link
Owner

spantaleev commented Jun 25, 2019

Once matrix-org/matrix-appservice-slack#143 gets merged, it looks like we should be able to add this to the configuration in matrix_appservice_slack_configuration_yaml:

dbdir: /data
hs_registration_file: ./registration.yaml

This assumes that we've done the split into a /data directory and a config directory (mounted at /usr/src/app/userconfig).

I guess we also depend on:

  • matrix-appservice-slack getting a new release (or at least having develop merged to master, which is probably what :latest is cut from)
  • the cadair/matrix-appservice-slack Docker image receiving an update (either :latest or :develop). Looks like the last update is from 8 months ago.

Since dbdir has ~always been a thing, we can probably start using it right now, without waiting for anything.

As for hs_registration_file, we can probably wait and see if something happens.. or just call our file slack-registration.yaml for now.

@Cadair
Copy link
Contributor

Cadair commented Jun 25, 2019

I hope to push a new docker image for testing this week or next, and hopefully there will be a release of the slack AS itself in the next couple of weeks as well.

@kingoftheconnors
Copy link
Contributor Author

kingoftheconnors commented Jun 25, 2019

Awesome. Be sure to drop a comment here to let us know when the docker update gets online!

I can take care of the re-integration for it.

@spantaleev
Copy link
Owner

According to matrix-org/matrix-appservice-slack#139, it looks like we may need to define homeserver.media_url as well.

@Cadair
Copy link
Contributor

Cadair commented Jun 30, 2019

Yes I think so.

@Cadair
Copy link
Contributor

Cadair commented Jun 30, 2019

I have pushed a new docker image with the tag "cadair", it folds in a lot of my open PRs at the moment. It should have the latest updates to specifying the registration path via the cli

@Cadair
Copy link
Contributor

Cadair commented Jul 2, 2019

matrix-org/matrix-appservice-slack#143 is merged fyi.

@kingoftheconnors
Copy link
Contributor Author

According to matrix-org/matrix-appservice-slack#139, it looks like we may need to define homeserver.media_url as well.

Would that be matrix.{{matrix_domain}} or just {{matrix_domain}}?

@kingoftheconnors
Copy link
Contributor Author

@Cadair is -p {{matrix_appservice_slack_matrix_port}} still needed?

@Cadair
Copy link
Contributor

Cadair commented Jul 2, 2019

Yes, you have to specify to the AS what port the HS is attempting to connect to.

@spantaleev
Copy link
Owner

According to matrix-org/matrix-appservice-slack#139, it looks like we may need to define homeserver.media_url as well.

Would that be matrix.{{matrix_domain}} or just {{matrix_domain}}?

It should be matrix.{{ matrix_domain }}.

@kingoftheconnors
Copy link
Contributor Author

So the bridge now can be reached, the proxied url (matrix.DOMAIN/appservice_slack) is reachable by the slack api, and I've been able to follow the directions by the appservice README, so a lot more progress has been made.

Right now there's a problem in linking channels, since the bridge doesn't seem to recognize when channels are linked and just says
Oops - SlackEventHandler failed: UnknownChannel { channel: 'CJVQ3ATT8' }
to all messages, regardless of which channel it's in.

@spantaleev
Copy link
Owner

I wonder if that's a bridge problem or something that's wrong with our configuration.
@Cadair?

@Cadair
Copy link
Contributor

Cadair commented Jul 15, 2019

I have also commented on the AS issue. Make sure that the bridge is getting events from synapse, specially the link commands.

@spantaleev
Copy link
Owner

spantaleev commented Jul 16, 2019

Looks like this might be fully functional now, given that networking has been fixed?

@kingoftheconnors
Copy link
Contributor Author

kingoftheconnors commented Jul 16, 2019

Networking has been fixed. There's a small to-do list left though:

  • I'm having trouble with Slack->Matrix messages. I need to get to the bottom of that first
  • I need to make a how-to in the docs file based on the Discord setup tutorial
  • I need to squash all the commits together to save space on the timeline

@kingoftheconnors
Copy link
Contributor Author

Also, the slack role doesn't implement the cadair image. I know a few of cadair's PRs have been pulled in, so do I still need to do that?

@kingoftheconnors
Copy link
Contributor Author

It works! Maybe try it out yourself, but if my bridge is anything to go off of, the slack role is functional!

... Although it still uses the :latest docker image rather than the :cadair one.

@Cadair
Copy link
Contributor

Cadair commented Jul 20, 2019

Now 0.3 is out I should rebuild the latest image. Although I should find out about the official docker image as well.

@Cadair
Copy link
Contributor

Cadair commented Jul 21, 2019

The latest tag of the docker image is now the master branch of the repo, which is the 0.3 release. So we should be good to go.

I have had a read over this and it looks great. I might try and deploy it on my personal server today.

@kingoftheconnors
Copy link
Contributor Author

It seems media_URL may be wrong, since the profile pics aren't being bridged. I'll be taking a look at this later today.

@kingoftheconnors
Copy link
Contributor Author

Alright, fixed! I just forgot to update my docker image. 😆

@spantaleev
Copy link
Owner

Seems like this is coming along nicely!

Thank you for all the hard work @kingoftheconnors and @Cadair!

@spantaleev
Copy link
Owner

Anything else that needs to be done before this gets merged?

@kingoftheconnors
Copy link
Contributor Author

Not right now. There's currently work for an official docker image for the appservice, but that probably won't be up for a while. Once that's up, I'll probably open a new PR for that one. This role works and I'm using it for my own server as we speak!

@Cadair
Copy link
Contributor

Cadair commented Aug 18, 2019 via email

@kingoftheconnors
Copy link
Contributor Author

Hm. Should we wait for that to stabilize or move over to that one now?

@Cadair
Copy link
Contributor

Cadair commented Aug 18, 2019 via email

@Cadair
Copy link
Contributor

Cadair commented Aug 20, 2019

I suggest we stay with this one until we get a stable release of the official image. I think this is ready to merge.

@spantaleev spantaleev merged commit 811a46a into spantaleev:master Aug 21, 2019
@spantaleev
Copy link
Owner

It's been a long ride, but I'm happy to finally merge this, so that everyone could start bridging to Slack easily!

Thank you for the great work and perseverance! 👍

Announced in dd0f355.

@kingoftheconnors kingoftheconnors changed the title WIP: Added Slack role Added Slack role Sep 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants