nifi icon indicating copy to clipboard operation
nifi copied to clipboard

NIFI-12411: Update PublishAMQP to read AMQP headers value from FlowFile attributes and `amq$headers` string

Open umarhussain15 opened this issue 2 years ago • 7 comments

Summary

NIFI-12411

Processor can now also create message headers using attributes matching the given Regex property. The name of the attribute is used as header key. In addition to that it can read values from two sources i.e. attributes matching Regex and the headers present in amq$headers string. In case of key duplication in both sources, the precedence property decide which value will be used.

AMQP attributes keys are now set as constants strings and reused where needed.

Added onScheduled to PublishAMQP and ConsumeAMQP to read all properties which don't change during running state.

Test cases added for PublishAMQP headers source changes.

Tracking

Please complete the following tracking steps prior to pull request creation.

Issue Tracking

Pull Request Tracking

  • [x] Pull Request title starts with Apache NiFi Jira issue number, such as NIFI-00000
  • [x] Pull Request commit message starts with Apache NiFi Jira issue number, as such NIFI-00000

Pull Request Formatting

  • [x] Pull Request based on current revision of the main branch
  • [x] Pull Request refers to a feature branch with one commit containing changes

Verification

Please indicate the verification steps performed prior to pull request creation.

Build

  • [x] Build completed using mvn clean install -P contrib-check
    • [x] JDK 21

Licensing

  • [x] New dependencies are compatible with the Apache License 2.0 according to the License Policy
  • [x] New dependencies are documented in applicable LICENSE and NOTICE files

Documentation

  • [x] Documentation formatting appears as expected in rendered files

umarhussain15 avatar Dec 02 '23 14:12 umarhussain15

@umarhussain15 Can you please rebase and consider the feedback shared from @exceptionfactory ?

Thanks

joewitt avatar May 04 '24 21:05 joewitt

Hi @joewitt @exceptionfactory, I have reverted the changes of making class level variables based on processor properties. Also, the branch is rebased on latest changes from main

umarhussain15 avatar May 12 '24 11:05 umarhussain15

@exceptionfactory @joewitt

I have added one more change, which will allow users to set amqp processor's password via env variables. I think this will be helpful in cases where we use AMQP processors in multiple NiFI instances. Instead of copying and pasting different passwords in processor properties, we can reference the env variable via its name. Let me know your thoughts on it. Thanks.

umarhussain15 avatar May 14 '24 19:05 umarhussain15

@exceptionfactory @joewitt

I have added one more change, which will allow users to set amqp processor's password via env variables. I think this will be helpful in cases where we use AMQP processors in multiple NiFI instances. Instead of copying and pasting different passwords in processor properties, we can reference the env variable via its name. Let me know your thoughts on it. Thanks.

Thanks for calling out this particular change @umarhussain15. Given the scope of the current pull request, I recommend considering that change separately.

In general, sensitive properties such as passwords do not support reading from environment variables. The reason is that environment variables are also accessible through Expression Language, so they are not necessarily secure. The use case is still supported through Parameter Contexts and Parameter Providers, where the Environment Variable Parameter Provider can be used. This is an additional layer of indirection, but it follows the general convention for other sensitive properties.

exceptionfactory avatar May 14 '24 20:05 exceptionfactory

Thanks for the explanation, @exceptionfactory. I will revert the change here then.

umarhussain15 avatar May 17 '24 12:05 umarhussain15

Reverted the AbstractAMQPProcessor change to support expression language in password field.

umarhussain15 avatar May 17 '24 14:05 umarhussain15

Checkstyle reported issues fixed

umarhussain15 avatar May 21 '24 09:05 umarhussain15

Hi @exceptionfactory, I have applied your suggestions and also removed the precedence option from the processor. I have also updated the additional documentation to describe the headers source setting of the processor.

umarhussain15 avatar Jul 20 '24 21:07 umarhussain15